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

[Henrik G Olsson via Phabricator via cfe-commits]

hnrklssn marked 2 inline comments as done.
hnrklssn added a comment.

Made changes in line with what @fcloutier suggested.



================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5321
 
+  SetLLVMFunctionAttributesForDefinition(D, Fn);
   CodeGenFunction(*this).GenerateCode(GD, Fn, FI);
----------------
fcloutier wrote:
> 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.
Indeed that approach works as well. There are some test cases that result in CurCodeDecl being NULL, like you suspected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137714