[PATCH] D137714: Do not merge traps in functions annotated optnone

Thu Nov 10 08:51:56 PST 2022

fcloutier requested changes to this revision.
fcloutier added inline comments.
This revision now requires changes to proceed.

================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5321

+  SetLLVMFunctionAttributesForDefinition(D, Fn);
   CodeGenFunction(*this).GenerateCode(GD, Fn, FI);
----------------
hnrklssn wrote:
> delcypher wrote:
> > I'm a little worried about this ordering change here. Could this have some unintended consequences?
> Yeah I was also a bit worried about that when making the change, since the functions are both quite broad and I'm not familiar with them from before. However it doesn't break any test cases, so I'm not sure what the consequences would be exactly.
> 
> For reference, also moving the call to setNonAliasAttributes so that it is still before the call to SetLLVMFunctionAttributesForDefinition breaks a ton of test cases so I'm somewhat hopeful that we have good test coverage for this type of change.
Could you get it from `CurCodeDecl->hasAttr<OptimizeNoneAttr>()` in CGExpr instead? Then you wouldn't have to change this.

Caveat: am not sure that `CurCodeDecl` is always set. For instance, do you have a `CurCodeDecl` when you generate a C++ static initializer? On the upside, if it's NULL, you can just bail out.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137714/new/

https://reviews.llvm.org/D137714