[PATCH] D69545: [globalisel][docs] Rework GMIR documentation and add an early GenericOpcode reference

Diana Picus via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 4 01:27:08 PST 2019


rovka accepted this revision.
rovka added a comment.
This revision is now accepted and ready to land.

The text itself LGTM.

FWIW I too would prefer generating this automatically (e.g. as is done for clang's AST Matchers) but I don't know the caveats involved in doing that. I suppose we could start with this and automate later (similarly to how we started with hand-written code and then moved to TableGen).

Maybe wait a couple of days to see if anyone feels strongly about automating.



================
Comment at: llvm/docs/GlobalISel/GenericOpcode.rst:270
+Add a scalar offset in addressible units to a pointer. Addressible units are
+typically bytes but this may vary between targets.
+
----------------
Actually I just noticed it says "bytes" here: https://github.com/llvm/llvm-project/blob/2be17087f8c38934b7fc9208ae6cf4e9b4d44f4b/llvm/include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h#L409

We should say the same thing in both places, either bytes or addressable units. Or ideally stop saying so much about the gMIR itself in the comments for the MachineIRBuilder.h.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69545





More information about the llvm-commits mailing list