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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
