Skip to content

Invalid IR when accessing union #224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
yugr opened this issue Aug 10, 2023 · 2 comments
Closed

Invalid IR when accessing union #224

yugr opened this issue Aug 10, 2023 · 2 comments
Assignees

Comments

@yugr
Copy link
Member

yugr commented Aug 10, 2023

This code

typedef union {                                                                 
  char x;                                                                       
  struct {                                                                      
    short h, l;                                                                 
  } b;                                                                          
} T;                                                                            
                                                                                
void foo(T reg) {                                                               
  reg.b.l;                                                                      
}                                                                               

currently generates invalid IR:

$ ~/src/clangir/build/bin/clang -fclangir-enable -emit-llvm -S tmp.c
loc("tmp.c":4:14): error: 'llvm.getelementptr' op index 1 indexing a struct is out of bounds

It seems that we are missing a bitcast in buildLValueForField and code even has a comment about this:

  // TODO(CIR): CodeGen requires a bitcast here for unions or for structs where 
  // the LLVM type doesn't match the desired type. No idea when the latter might
  // occur, though.                                                             

Indeed adding

  if (rec->isUnion()) {
    auto memTy = getTypes().convertTypeForMem(FieldType);
    addr = builder.createElementBitCast(getLoc(field->getSourceRange()), addr, memTy);
  }

fixes the problem.

@sitio-couto
Copy link
Collaborator

@yugr thanks for tracking this.

I'm currently working on refactoring how unions work in CIR. It will likely fix this issue.

@sitio-couto sitio-couto self-assigned this Aug 10, 2023
@bcardosolopes
Copy link
Member

Thanks for sharing @yugr!

sitio-couto added a commit that referenced this issue Aug 11, 2023
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

[ghstack-poisoned]
sitio-couto added a commit that referenced this issue Aug 22, 2023
…ck all members"

This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

[ghstack-poisoned]
sitio-couto added a commit that referenced this issue Aug 22, 2023
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

[ghstack-poisoned]
sitio-couto added a commit that referenced this issue Aug 23, 2023
…ck all members"

This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

[ghstack-poisoned]
sitio-couto added a commit that referenced this issue Aug 23, 2023
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

[ghstack-poisoned]
sitio-couto added a commit that referenced this issue Aug 28, 2023
…ck all members"

This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

[ghstack-poisoned]
sitio-couto added a commit that referenced this issue Aug 28, 2023
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

[ghstack-poisoned]
lanza pushed a commit that referenced this issue Oct 27, 2023
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

ghstack-source-id: 8a97542
Pull Request resolved: #229
lanza pushed a commit that referenced this issue Dec 20, 2023
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

ghstack-source-id: 8a97542
Pull Request resolved: #229
lanza pushed a commit that referenced this issue Jan 29, 2024
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

ghstack-source-id: 8a97542
Pull Request resolved: #229
lanza pushed a commit that referenced this issue Mar 23, 2024
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

ghstack-source-id: 8a97542
Pull Request resolved: #229
eZWALT pushed a commit to eZWALT/clangir that referenced this issue Mar 24, 2024
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes llvm#224

ghstack-source-id: 8a97542
Pull Request resolved: llvm#229
eZWALT pushed a commit to eZWALT/clangir that referenced this issue Mar 24, 2024
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes llvm#224

ghstack-source-id: 8a97542
Pull Request resolved: llvm#229
lanza pushed a commit that referenced this issue Apr 29, 2024
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

ghstack-source-id: 8a97542
Pull Request resolved: #229
lanza pushed a commit that referenced this issue Apr 29, 2024
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

ghstack-source-id: 8a97542
Pull Request resolved: #229
eZWALT pushed a commit to eZWALT/clangir that referenced this issue Apr 29, 2024
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes llvm#224

ghstack-source-id: 8a97542
Pull Request resolved: llvm#229
lanza pushed a commit that referenced this issue Apr 29, 2024
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

ghstack-source-id: 8a97542
Pull Request resolved: #229
bruteforceboy pushed a commit to bruteforceboy/clangir that referenced this issue Oct 2, 2024
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes llvm#224

ghstack-source-id: 8a97542
Pull Request resolved: llvm#229
Hugobros3 pushed a commit to shady-gang/clangir that referenced this issue Oct 2, 2024
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes llvm#224

ghstack-source-id: 8a97542
Pull Request resolved: llvm#229
keryell pushed a commit to keryell/clangir that referenced this issue Oct 19, 2024
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes llvm#224

ghstack-source-id: 8a97542
Pull Request resolved: llvm#229
lanza pushed a commit that referenced this issue Nov 5, 2024
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

ghstack-source-id: 8a97542
Pull Request resolved: #229
lanza pushed a commit that referenced this issue Mar 18, 2025
This diverges from the original codegen by tracking all members of a
union, instead of just the largest one. This is necessary to support
type-checking at the MLIR level when accessing union members. It also
preserves more information about the source code, which might be useful.

Fixes #224

ghstack-source-id: 8a97542
Pull Request resolved: #229
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants