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

[Diana Picus via Phabricator via llvm-commits]

rovka added inline comments.


================
Comment at: llvm/docs/GlobalISel/GMIR.rst:93
+just physically stored apart and each instruction can only access register A or
+register B but never both. If we want to perform an operation on data that's in
+both register files, we must first copy all the data into one register file.
----------------
I'd suggest "can only access registers from A or registers from B, but never a mix of the two. If we want to perform an operation on data that's split between the register files, we must first copy all of the data into just one of them."


================
Comment at: llvm/docs/GlobalISel/GMIR.rst:97
+Given a processor like this, we would benefit from clustering related data
+together into one register file so that we minimize the cost of copying data
+around. Register Banks are a means to do bind a particular virtual register to
----------------
I would suggest "the cost of copying data around between different instructions that use it" (or some other way that highlights that we're looking at more that one instruction at a time).


================
Comment at: llvm/docs/GlobalISel/GMIR.rst:98
+together into one register file so that we minimize the cost of copying data
+around. Register Banks are a means to do bind a particular virtual register to
+one of these register files in order to constrain the register allocator to use
----------------
"to bind"


================
Comment at: llvm/docs/GlobalISel/GMIR.rst:100
+one of these register files in order to constrain the register allocator to use
+this particular register file.
+
----------------
You say register 5 times in this sentence :)

"Register Banks are a means to constrain the register allocator to use a particular register file for a given virtual register." would be a bit more straightforward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69545