[llvm-branch-commits] [clang] [llvm] [KeyInstr][Clang] Add ApplyAtomGroup (PR #134632)

[Orlando Cazalet-Hyams via llvm-branch-commits]

OCHyams wrote:

> This all seems fine -- I guess the plumbing here has to get in without a test, before then later real changes come in and can be tested.
> 
> The "Override" vs new-source-atom distinction seems a little clunky, although I haven't read how it's used to get the full context. IMO it's worth putting thought into a better name: can we pick an abstract name describing the purpose ("New key operation"?) where the override thing is just an implementation feature?

Hmm you're right it's not very clear. The "override" situation is needed because of how returns are handled in Clang (ret atoms review - #134652), which is that multiple returns are emitted as branches to a single return-block. Those branches get the source location info for the return, and the actual ret is associated with the closing brace. However, if there's only one pred to the return block it's folded into it, and the return takes the source location of the branch. I can't remember exactly why the atom application had to be structured in this slightly convoluted way (should've written better comments!). I'll see if it can be simplified a bit, and if not I'll rename the functions and add better comments.


https://github.com/llvm/llvm-project/pull/134632