[PATCH] D154766: [GlobalISel] convergent intrinsics

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp:792
+  auto Attrs = Intrinsic::getAttributes(getContext(), ID);
+  auto MIB = buildInstr(
+      getIntrinsicCode(HasSideEffects, Attrs.hasFnAttr(Attribute::Convergent)));
----------------
sameerds wrote:
> arsenm wrote:
> > I'd expect this to use another bool parameter along with HasSideEffects.
> > 
> > I'm also thinking ahead to noconvergent call sites
> If you mean a parameter from the TableGen description, then this is the place where the `isConvergent` parameter from the def of the intrinsic gets translated to the `isConvergent` parameter in the def of `G_INTRINSIC_CONVERGENT_*`. The use of `getFnAttributes` on the intrinsic declaration is how we access the former, and the opcode selection inside `getIntrinsicCode` is how we set the latter.
I mean buildIntrinsic has a HasSideEffects bool parameter. I expect another partner operand for convergent, rather than looking it up through the declaration here. A different buildIntrinsic overload that queries the underlying declaration is potentially useful, but I don't want to mix the two here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154766