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

[Zequan Wu via Phabricator via cfe-commits]

zequanwu added inline comments.


================
Comment at: clang/test/CodeGen/attr-nomerge.cpp:44
+
+  [[clang::nomerge]] __builtin_trap();
 }
----------------
hans wrote:
> Maybe do __debugbreak() too since that's also mentioned on the debug.
The `__debugbreak()` is also emitted from `EmitTrapCall()`. So, just add a test case for `__debugbreak()`. 


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:6856
       return;
     }
     TargetLowering::ArgListTy Args;
----------------
hans wrote:
> Do we need to handle this "else" branch too?
> 
> Or would it make sense to do the nomerge check in the caller of `visitIntrinsicCall` instead?
Setting CLI.NoMerge instead of setting DAG.getRoot(), because it later somehow replace the root with another instruction which causes the call instruction to lose the attribute. 

Added a test case for else branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146164