[CIR][CodeGen] Support static local variables #1224
Conversation
Created using spr 1.3.5
| return $_attr.getAst()->getLocation(); | ||
| }] | ||
| >, | ||
| InterfaceMethod<"", "const clang::VarDecl *", "getRawDecl", (ins), [{}], |
There was a problem hiding this comment.
@bcardosolopes Thoughts on this? This wasn't scaling to the real world. e.g. functions that take a VarDecl as an argument.
There was a problem hiding this comment.
After you latest update you can probably ditch this altogether
| // CHECK: declare i32 @__cxa_guard_acquire(ptr) | ||
| // CHECK: declare void @__cxa_guard_release(ptr) | ||
|
|
||
| // CHECK: define dso_local void @_Z3foov() |
There was a problem hiding this comment.
Maybe add a FIXME in the testcase?
Created using spr 1.3.5
|
Thanks so much for working on this, I gotta say this PR is huge though, we should probably talk about a way to split this down a bit, maybe we can incrementally converge faster if this is done in pieces. |
Created using spr 1.3.5
Created using spr 1.3.5
Created using spr 1.3.5
Created using spr 1.3.5
lanza
changed the title
Created using spr 1.3.5
Created using spr 1.3.5
lanza
changed the title
| // RUN: %clang_cc1 -triple aarch64-none-linux-android21 -fclangir -emit-cir -clangir-disable-passes %s -o %t.cir | ||
| // RUN: FileCheck --input-file=%t.cir %s --check-prefix=CIRGEN | ||
| // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-llvm %s -o %t.cir | ||
| // RUN: FileCheck --input-file=%t.cir %s --check-prefix=LLVM |
There was a problem hiding this comment.
Can you update the testcase to reflect what you can currently emit? It's fine if you want to leave the other things around for now, just add TODOs or FIXMEs.
There was a problem hiding this comment.
Looks like you already did, nevermind!
There was a problem hiding this comment.
Actually, I got confused because lowering prepare changes were so big I they got folded by github. Reinstating my point: please remove the lowering prepare changes from this PR and change the testcase to only cover CIRGen. I think there's more we can move to CIRGen that don't require all those interfaces (I asked for the threadsafe attribute in the other review but somehow you've missed addressing that)
There was a problem hiding this comment.
After ditching interfaces and leaving lowering prepare for another PR, this mostly looks good to me (with the other minor changes requested)
| void emitInvariantStart(CharUnits Size); | ||
|
|
||
| /// emitCXXGlobalVarDeclInit - Create the initializer for a C++ variable with |
There was a problem hiding this comment.
Remove the emitCXXGlobalVarDeclInit -
Support static local variable. Emit the initialization code into the
ctor region of the GlobalOp that represents the static local. During
LoweringPrepare expand that ctor region into the guardAcquire/release
walk.
Follow ups need to support exceptions and guardAbort.