[PATCH] D11644: Emit <regmask R1 R2 R3 ...> instead of just <regmask> in IR dumps.

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 13 09:04:34 PDT 2015


qcolombet added inline comments.

================
Comment at: lib/CodeGen/MachineInstr.cpp:415
@@ +414,3 @@
+      unsigned MaskBit = i % 32;
+      if (getRegMask()[MaskWord] & (1 << MaskBit))
+        OS << " " << PrintReg(i, TRI);
----------------
dsanders wrote:
> qcolombet wrote:
> > I would have expected the loop to look a bit differently to avoid the divide and modulo:
> > const uint32_t *MaskWord = getRegMask();
> > unsigned BaseIdx = 0;
> > for (; MaskWord; ++MaskWord, BaseIdx += 32) {
> >   for (unsigned Idx = BaseIdx; Offset < BaseIdx + 32; ++Offset)
> >     OS << " " << PrintReg(Idx, TRI);
> > }
> A good compiler will reduce the divide and modulo to 'x>>5' and 'x&31', but I thought it was clearer to use 'x/32' and 'x%32' in the source.
> 
> >  const uint32_t *MaskWord = getRegMask();
> >  unsigned BaseIdx = 0;
> >  for (; MaskWord; ++MaskWord, BaseIdx += 32) {
> >    for (unsigned Idx = BaseIdx; Offset < BaseIdx + 32; ++Offset)
> >      OS << " " << PrintReg(Idx, TRI);
> >  }
> I don't think this loop works correctly. The array can have runs of zero words inside it and is not guaranteed to end in a zero word.
> I don't think this loop works correctly. The array can have runs of zero words inside it and is not guaranteed to end in a zero word.

Of course, you’re right. I must have mixed that with another API :).


http://reviews.llvm.org/D11644





More information about the llvm-commits mailing list