[PATCH] D146164: Fix nomerge attribute not working with __builtin_trap().

Thu Mar 16 13:55:23 PDT 2023

zequanwu added a comment.

In D146164#4200414 <https://reviews.llvm.org/D146164#4200414>, @rsmith wrote:

> FYI: this attribute appears to not work on (at least) x86 and arm currently, because the backend also does some merging: https://godbolt.org/z/d43M83oax

Thanks for reporting it. I'll look into it later.

================
Comment at: clang/lib/CodeGen/CGExpr.cpp:3626-3627
   }
-
+  if (InNoMergeAttributedStmt)
+    TrapCall->addFnAttr(llvm::Attribute::NoMerge);
   return TrapCall;
----------------
rsmith wrote:
> There are 496 calls to `Builder.CreateCall` in clang's `CodeGen`. Do they all need this change? If not, how can we be confident we've found all the ones that do? (From a quick check, at least MSVC's `__fastfail` builtin seems like it would also benefit from this handling.)
> 
> Would it be reasonable to make clang's `CGBuilder` do this for every call instruction we create?
I think all of them (builtin functions) should have that change, theoretically, because they are not different from `__builtin_trap`. Probably we should change the `Builder.CreateCall` to take a list of attributes that will be added to that CallInst and update all calls to `CreateCall`. Is there any easier approach?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146164