[PATCH] D67397: [RISCV] Add MachineInstr immediate verification

[Sam Elliott via Phabricator via llvm-commits]

lenary added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:477
+  unsigned i = 0;
+  for (auto &OI : Desc.operands()) {
+    unsigned OpType = OI.OperandType;
----------------
jrtc27 wrote:
> Combining foreach and an index like this is a bit ugly due to having the counter increment at the end (and it's brittle if someone introduces a `continue` into the loop for whatever reason). AMDGPU takes the alternate approach of `for (int i = 0, e = Desc.getNumOperands(); i != e; ++i)`; then, the only change you need to the body is `unsigned OpType = Desc.OpInfo[i].OperandType`. Alternatively you can keep track of the start, current and end iterators, calculating the index by subtracting the current from the start, or just expand out the foreach and include `i` in the for loop itself.
There's an `enumerate` function in STLExtras.h that will give you back a pair of the index and the object in the iterator. I think that would probably be even cleaner. (See comment at about line 1485 in STLExtras.h)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67397