Skip to content
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

Invalid LLVM for constant array reference #352

Closed
yugr opened this issue Dec 13, 2023 · 3 comments
Closed

Invalid LLVM for constant array reference #352

yugr opened this issue Dec 13, 2023 · 3 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@yugr
Copy link
Member

yugr commented Dec 13, 2023

Compiling the following code with trunk CIR

int a[16];
int *const p = a;

void foo() {
  p[0];
}

generates invalid LLVM IR:

loc("tmp.c":5:3): error: 'llvm.mlir.constant' op only supports integer, float, string or elements attributes

Removal of const fixes it.

@yugr
Copy link
Member Author

yugr commented Dec 13, 2023

The issue is caused by

%0 = llvm.mlir.constant(#cir.global_view<@a> : !cir.ptr<!cir.int<s, 32>>) : !llvm.ptr

which should have been llvm.mlir.addressof instead. This is caused by generation of

%0 = cir.const(#cir.global_view<@a> : !cir.ptr<!s32i>)

rather than cir.get_global in ScalarExprEmitter::VisitDeclRefExpr:

    if (CIRGenFunction::ConstantEmission Constant = CGF.tryEmitAsConstant(E)) {
      return CGF.buildScalarConstant(Constant, E);
    }

@bcardosolopes bcardosolopes added bug Something isn't working good first issue Good for newcomers labels Dec 13, 2023
@yugr
Copy link
Member Author

yugr commented Dec 14, 2023

A simple workaround is to handle this at lowering stage:

diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 076c0eb814b4..8b23213c70f2 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -1100,6 +1100,13 @@ public:
       return op.emitError() << "unsupported constant type " << op.getType();
 
+    if (auto GV = dyn_cast<mlir::cir::GlobalViewAttr>(attr)) {
+      auto newOp = lowerCirAttrAsValue(op, GV, rewriter, getTypeConverter());
+      rewriter.replaceOp(op, newOp);
+      return mlir::success();
+    }
+
     rewriter.replaceOpWithNewOp<mlir::LLVM::ConstantOp>(
         op, getTypeConverter()->convertType(op.getType()), attr);

but I suspect we may not want to generate the problematic cir.const at all.

bcardosolopes added a commit that referenced this issue Jan 4, 2024
…-352) (#363)

The error manifested in code like
```
int a[16];
int *const p = a;

void foo() {
  p[0];
}
```
It's one the most frequent errors in current llvm-test-suite.

I've added the test to globals.cir which is currently XFAILed, I think
@gitoleg will fix it soon.

Co-authored-by: Bruno Cardoso Lopes <[email protected]>
@yugr
Copy link
Member Author

yugr commented Jan 9, 2024

Fixed by #363

@yugr yugr closed this as completed Jan 9, 2024
lanza pushed a commit that referenced this issue Jan 29, 2024
…-352) (#363)

The error manifested in code like
```
int a[16];
int *const p = a;

void foo() {
  p[0];
}
```
It's one the most frequent errors in current llvm-test-suite.

I've added the test to globals.cir which is currently XFAILed, I think
@gitoleg will fix it soon.

Co-authored-by: Bruno Cardoso Lopes <[email protected]>
lanza pushed a commit that referenced this issue Mar 23, 2024
…-352) (#363)

The error manifested in code like
```
int a[16];
int *const p = a;

void foo() {
  p[0];
}
```
It's one the most frequent errors in current llvm-test-suite.

I've added the test to globals.cir which is currently XFAILed, I think
@gitoleg will fix it soon.

Co-authored-by: Bruno Cardoso Lopes <[email protected]>
eZWALT pushed a commit to eZWALT/clangir that referenced this issue Mar 24, 2024
…vmgh-352) (llvm#363)

The error manifested in code like
```
int a[16];
int *const p = a;

void foo() {
  p[0];
}
```
It's one the most frequent errors in current llvm-test-suite.

I've added the test to globals.cir which is currently XFAILed, I think
@gitoleg will fix it soon.

Co-authored-by: Bruno Cardoso Lopes <[email protected]>
eZWALT pushed a commit to eZWALT/clangir that referenced this issue Mar 24, 2024
…vmgh-352) (llvm#363)

The error manifested in code like
```
int a[16];
int *const p = a;

void foo() {
  p[0];
}
```
It's one the most frequent errors in current llvm-test-suite.

I've added the test to globals.cir which is currently XFAILed, I think
@gitoleg will fix it soon.

Co-authored-by: Bruno Cardoso Lopes <[email protected]>
lanza pushed a commit that referenced this issue Apr 29, 2024
…-352) (#363)

The error manifested in code like
```
int a[16];
int *const p = a;

void foo() {
  p[0];
}
```
It's one the most frequent errors in current llvm-test-suite.

I've added the test to globals.cir which is currently XFAILed, I think
@gitoleg will fix it soon.

Co-authored-by: Bruno Cardoso Lopes <[email protected]>
lanza pushed a commit that referenced this issue Apr 29, 2024
…-352) (#363)

The error manifested in code like
```
int a[16];
int *const p = a;

void foo() {
  p[0];
}
```
It's one the most frequent errors in current llvm-test-suite.

I've added the test to globals.cir which is currently XFAILed, I think
@gitoleg will fix it soon.

Co-authored-by: Bruno Cardoso Lopes <[email protected]>
eZWALT pushed a commit to eZWALT/clangir that referenced this issue Apr 29, 2024
…vmgh-352) (llvm#363)

The error manifested in code like
```
int a[16];
int *const p = a;

void foo() {
  p[0];
}
```
It's one the most frequent errors in current llvm-test-suite.

I've added the test to globals.cir which is currently XFAILed, I think
@gitoleg will fix it soon.

Co-authored-by: Bruno Cardoso Lopes <[email protected]>
lanza pushed a commit that referenced this issue Apr 29, 2024
…-352) (#363)

The error manifested in code like
```
int a[16];
int *const p = a;

void foo() {
  p[0];
}
```
It's one the most frequent errors in current llvm-test-suite.

I've added the test to globals.cir which is currently XFAILed, I think
@gitoleg will fix it soon.

Co-authored-by: Bruno Cardoso Lopes <[email protected]>
bruteforceboy pushed a commit to bruteforceboy/clangir that referenced this issue Oct 2, 2024
…vmgh-352) (llvm#363)

The error manifested in code like
```
int a[16];
int *const p = a;

void foo() {
  p[0];
}
```
It's one the most frequent errors in current llvm-test-suite.

I've added the test to globals.cir which is currently XFAILed, I think
@gitoleg will fix it soon.

Co-authored-by: Bruno Cardoso Lopes <[email protected]>
Hugobros3 pushed a commit to shady-gang/clangir that referenced this issue Oct 2, 2024
…vmgh-352) (llvm#363)

The error manifested in code like
```
int a[16];
int *const p = a;

void foo() {
  p[0];
}
```
It's one the most frequent errors in current llvm-test-suite.

I've added the test to globals.cir which is currently XFAILed, I think
@gitoleg will fix it soon.

Co-authored-by: Bruno Cardoso Lopes <[email protected]>
keryell pushed a commit to keryell/clangir that referenced this issue Oct 19, 2024
…vmgh-352) (llvm#363)

The error manifested in code like
```
int a[16];
int *const p = a;

void foo() {
  p[0];
}
```
It's one the most frequent errors in current llvm-test-suite.

I've added the test to globals.cir which is currently XFAILed, I think
@gitoleg will fix it soon.

Co-authored-by: Bruno Cardoso Lopes <[email protected]>
lanza pushed a commit that referenced this issue Nov 5, 2024
…-352) (#363)

The error manifested in code like
```
int a[16];
int *const p = a;

void foo() {
  p[0];
}
```
It's one the most frequent errors in current llvm-test-suite.

I've added the test to globals.cir which is currently XFAILed, I think
@gitoleg will fix it soon.

Co-authored-by: Bruno Cardoso Lopes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants