[PATCH] D69498: IR: Invert convergent attribute handling

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Requiring the presence of an attribute for correctness is a bad thing. OpenMP was missing this annotation in a number of places and is probably still missing it elsewhere. I wouldn't bet on handwritten bitcode libraries getting it right everywhere either. An optimisation pass accidentally dropping the attribute seems a plausible failure mode as well.

Strongly in favour of replacing convergent with no{n}convergent in the IR.

Not as convinced it should be inserted by the front end. The attribute is needed before any CFG rewrites and as far as I know they all occur downstream of the front end. That suggests an IR pass that walks over all functions, intrinsic calls, inline asm and so forth and marks them as appropriate. Standard C++/x86 and similar don't need to run the pass, OpenCL/x86 probably does. I'd suggest running it manually across handwritten bitcode as a sanity check as well.

Of course, //if// a front end targeting gpus wants to change control flow (julia may do this, mlir does, I sincerely hope clang doesn't) //and// use this attribute to control the process, then that front end picks up the responsibility for inserting the attribute where it wants it.


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

https://reviews.llvm.org/D69498