Fix nomerge attribute not working with __builtin_trap().

[zequanwu]

Summary

1. It fixes the problem that llvm.trap() not getting the nomerge attribute.
2. It sets nomerge flag for the node if the instruction has nomerge arrtibute.

Fixes #53011

[hans]

Since we're touching SelectionDAG, does GlobalISel also need updating?

clang/test/CodeGen/attr-nomerge.cpp

44

Maybe do __debugbreak() too since that's also mentioned on the debug.

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

6853

(I guess you could have used DAG.getRoot() instead of creating the Node variable.)

6856

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?

[zequanwu]

• Add test case for __debugbreak().
• Handle named trap instructions and add test case for it.

clang/test/CodeGen/attr-nomerge.cpp

44

The __debugbreak() is also emitted from EmitTrapCall(). So, just add a test case for __debugbreak().

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

6856

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.

[rsmith]

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

clang/lib/CodeGen/CGExpr.cpp

3626–3628

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?

[zequanwu]

In 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.

clang/lib/CodeGen/CGExpr.cpp

3626–3628

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?

clang/lib/CodeGen/CGExpr.cpp

llvm::CallInst *CodeGenFunction::EmitTrapCall(llvm::Intrinsic::ID IntrID) {
   llvm::CallInst *TrapCall =
       Builder.CreateCall(CGM.getIntrinsic(IntrID));
 
   if (!CGM.getCodeGenOpts().TrapFuncName.empty()) {
     auto A = llvm::Attribute::get(getLLVMContext(), "trap-func-name",
                                   CGM.getCodeGenOpts().TrapFuncName);
     TrapCall->addFnAttr(A);
   }
-
+  if (InNoMergeAttributedStmt)
+    TrapCall->addFnAttr(llvm::Attribute::NoMerge);
   return TrapCall;
 }
 
 Address CodeGenFunction::EmitArrayToPointerDecay(const Expr *E,
                                                  LValueBaseInfo *BaseInfo,
                                                  TBAAAccessInfo *TBAAInfo) {
   assert(E->getType()->isArrayType() &&
          "Array to pointer decay must have array source type!");
 

clang/test/CodeGen/attr-nomerge.cpp

void foo(int i, A *ap, B *bp) {
   a.g();
   A::f1();
 
   B b;
   b.g();
 
   A *newA = new B();
   delete newA;
+
+  [[clang::nomerge]] __builtin_trap();
 }
 
 int g(int i);
 
 void something() {
   g(1);
 }
 

 // CHECK: call void @_ZN1A2f1Ev() #[[ATTR0]]
 // CHECK: call void @_ZN1BC1Ev({{.*}}){{$}}
 // CHECK: call void @_ZN1B1gEv({{.*}}){{$}}
 // CHECK: call void @_ZN1BC1Ev({{.*}}){{$}}
 // CHECK: load ptr, ptr
 // CHECK: load ptr, ptr
 // CHECK: %[[AG:.*]] = load ptr, ptr
 // CHECK-NEXT: call void %[[AG]](ptr {{.*}}) #[[ATTR1]]
+// CHECK: call void @llvm.trap() #[[ATTR0]]
 // CHECK: call void  @_ZN1AD1Ev(ptr {{.*}}) #[[ATTR1]]
 
 // CHECK-DAG: attributes #[[ATTR0]] = {{{.*}}nomerge{{.*}}}
 // CHECK-DAG: attributes #[[ATTR1]] = {{{.*}}nomerge{{.*}}}

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

case Intrinsic::expect:
     return;
 
   case Intrinsic::ubsantrap:
   case Intrinsic::debugtrap:
   case Intrinsic::trap: {
     StringRef TrapFuncName =
         I.getAttributes().getFnAttr("trap-func-name").getValueAsString();
     if (TrapFuncName.empty()) {
+      SDValue Node;
       switch (Intrinsic) {
       case Intrinsic::trap:
-        DAG.setRoot(DAG.getNode(ISD::TRAP, sdl, MVT::Other, getRoot()));
+        Node = DAG.getNode(ISD::TRAP, sdl, MVT::Other, getRoot());
         break;
       case Intrinsic::debugtrap:
-        DAG.setRoot(DAG.getNode(ISD::DEBUGTRAP, sdl, MVT::Other, getRoot()));
+        Node = DAG.getNode(ISD::DEBUGTRAP, sdl, MVT::Other, getRoot());
         break;
       case Intrinsic::ubsantrap:
-        DAG.setRoot(DAG.getNode(
+        Node = DAG.getNode(
             ISD::UBSANTRAP, sdl, MVT::Other, getRoot(),
             DAG.getTargetConstant(
                 cast<ConstantInt>(I.getArgOperand(0))->getZExtValue(), sdl,
-                MVT::i32)));
+                MVT::i32));
         break;
       default: llvm_unreachable("unknown trap intrinsic");
       }
+      DAG.addNoMergeSiteInfo(Node.getNode(), I.hasFnAttr(Attribute::NoMerge));
+      DAG.setRoot(Node);
       return;
     }
     TargetLowering::ArgListTy Args;
     if (Intrinsic == Intrinsic::ubsantrap) {
       Args.push_back(TargetLoweringBase::ArgListEntry());
       Args[0].Val = I.getArgOperand(0);
       Args[0].Node = getValue(Args[0].Val);
       Args[0].Ty = Args[0].Val->getType();
     }
 

llvm/test/CodeGen/X86/nomerge.ll

 
 if.end3:
   tail call void @bar() #0
   ret void
 }
 
 declare dso_local void @bar()
 
+define void @nomerge_trap(i32 %i) {
+entry:
+  switch i32 %i, label %if.end3 [
+    i32 5, label %if.then
+    i32 7, label %if.then2
+  ]
+
+if.then:
+  tail call void @llvm.trap() #0
+  br label %if.end3
+
+if.then2:
+  tail call void @llvm.trap() #0
+  br label %if.end3
+
+if.end3:
+  tail call void @llvm.trap() #0
+  ret void
+}
+
+declare dso_local void @llvm.trap()
+
 attributes #0 = { nomerge }
 
 ; CHECK-LABEL: foo:
 ; CHECK: # %bb.0: # %entry
 ; CHECK: # %bb.1: # %entry
 ; CHECK: # %bb.2: # %if.then
 ; CHECK-NEXT: callq bar
 ; CHECK: jmp bar # TAILCALL
 ; CHECK: .LBB0_3: # %if.then2
 ; CHECK: callq bar
 ; CHECK: .LBB0_4: # %if.end3
 ; CHECK: jmp bar # TAILCALL
+
+; CHECK-LABEL: nomerge_trap:
+; CHECK:      # %bb.0: # %entry
+; CHECK:      # %bb.1: # %entry
+; CHECK:      # %bb.2: # %if.then
+; CHECK-NEXT: ud2
+; CHECK-NEXT: ud2
+; CHECK-NEXT: retq
+; CHECK-NEXT: .LBB1_3: # %if.then2
+; CHECK-NEXT: ud2
+; CHECK-NEXT: .LBB1_4: # %if.end3
+; CHECK-NEXT: ud2
+; CHECK-NEXT: retq