Potential bug in CombToArith Pass?

Incubator CIRCT
1

Hi there!

I am reusing the CombToArith Pass from the LLHD/arcilator pipeline.

I notice in the conversion pattern for an ExtractOp:

    Value lowBit = rewriter.create<arith::ConstantOp>(
        op.getLoc(),
        IntegerAttr::get(adaptor.getInput().getType(), adaptor.getLowBit()));
    Value shifted =
        rewriter.create<ShRUIOp>(op.getLoc(), adaptor.getInput(), lowBit);
    rewriter.replaceOpWithNewOp<TruncIOp>(op, op.getResult().getType(),
                                          shifted);

That the input bits are shifted by lowBit. I may have misunderstood, but shouldn’t this shift the value by lowBit + 1 ? Since I assume we want to shift by the number of bits extracted ?

e.g. comb.extract %word from 0 would not shift at all.

2

Hi @Nmjfry,

The value is shifted by lowBit because the bit at index lowBit should be included in the slice. It’s expected that comb.extract %word from 0 is not shifted to the right because there is no bit to be cut off.

Consider the following 16 bit integer %value:

15 14 13 12 11 10  9  8  7  6  5  4  3  2  1  0 
-------------------------------------------------
| MSB                                       LSB | 16 bit integer vector
-------------------------------------------------

Let’s consider you have an operation %res = comb.extract %value from 2 : (i16) -> i4:
%res should contain the bits with indices 2, 3, 4, and 5. To end up with this, we shift by 2 (= lowBit) to the right, then truncate to an i4.

If we would shift by lowBit + 1, we would get bits 3, 4, 5, and 6, i.e., the bit at index lowBit would not be included.

1 Like
3

Ahh I had indeed misunderstood, thanks!

comb.extract (::circt::comb::ExtractOp):

“Extract a range of bits into a smaller value, lowBit specifies the lowest bit included.”

Should have read this more carefully…