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

6852

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

6855

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

6855

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–3627

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–3627

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?