@Prabhuk This reproduces with my setup as well. Did you try building with -DBUILD_SHARED_LIBS=ON?

Yep, I'm about to revert it.

6cb3866b1ce9d835402e414049478cea82427cf1

Yep, I'm about to revert it.

Yep, I'm about to revert it.

Rebased.

Rebased.

Yes it looks fine but I would not call it a "fix" since there is no bug here.

Is there any test that verifies that -mattr=-real-true16 is the default? If so, LGTM

Update decodeSrcOp() to use decodeNonVGPRSrcOp().

@Joe_Nash Joe, do you still request changes for this?

Ping.

Ping.

Ping.

Ping.

Add a comment for the predicates.

Remove -mattr=-flat-for-global.

Do we really need both? Are they testing different things?

I mean like for wave32, we have test/CodeGen/AMDGPU/wave32.ll. It's not completely comprehensive, but it kept testing isolated when wave32 support was first added. At this point a lot of what's in it is probably redundant and duplicated in other tests

@foad Jay, does this still LGTY after the rework?

Rebased on the current top of tree. Remove dependency on D157088.

Let's postpone this until the benefit is more obvious.

Personally I don't see much benefit to the "make no references to encodings" abstraction unless you are planning to change things, such that the interval endpoints are no longer register encoding values. Are you?

But I really wanted to understand whether it was *intended* to change any behaviour.

Jay, do you suggest this should be marked NFC?

Is this NFC? So long as the two pieces of code you updated are consistent, there is no bug, is there?

Update the commit description.

Sorry, that's unintentionally!

Update the commit description.

The hi/lo bit was specifically chosen to be the LSB in the Register encoding so that subtracting registers creates a logical register range. This is used for True16 codegen support SIInsertWaitcnts, and likely elsewhere.

Restore the non-True16 case branch to then remove it with a separate patch.

The encoding is a bit different from what we use downstream, because I don't like the idea of assigning the is-high flag to the LSB. This makes encodings that we get from getEncodingValue() for the non-16bit registers effectively incorrect and thus requiring additional treatment. Placing the flag in one of the MSBs feels like it extends the existing encoding conventions more naturally.

Update to add bits necessary for disassembling .h registers.

Update to add bits necessary for disassembling .h registers.

Thanks!

Ping.

Change the commit title as suggested and remove the unused code handling
the non-True16 case along with the test covering it.

Disassembler tests went to D156782.

Update to use the new feature name.