[PATCH] D64726: AMDGPU/GlobalISel: Fix not constraining result reg of copies to VCC

Thu Jul 18 07:34:20 PDT 2019

arsenm added a comment.

In D64726#1591516 <https://reviews.llvm.org/D64726#1591516>, @nhaehnle wrote:

> But then following this logic, I still think that by analogy with G_ZEXT, the operation of `COPY from s1 into vcc` should have the semantics of ignoring the high bits of the "s1 which is really an s32". Since there's nothing in the MIR test which guarantees that the incoming high bits of $sgpr0 are 0, the resulting code needs to have some form of masking.
>
> (As an aside, there's an argument to be made that the resulting code should really be an S_AND_B32 with 1 followed by an S_CSELECT_B64 writing to vcc instead of using the VALU, but we should probably clear up the semantics/correctness question first.)

For the purpose of the test, it doesn't matter what is really correct. It just needs to get an s1 value in a non-condition register bank. Theoretically there could have been an AssertZext equivalent, which was correctly deleted along with the G_AND the legalizer produced, turning it into a plain argument copy. The selector shouldn't need to be second guessing what the legalizer/combiner did was semantically correct.

An alternative strategy I've been considering is to disallow SGPR/VGPR s1 values entirely, and to always require these to use SCC/VCC producers for them (which would be a lot simpler). Any s1 use would then get a COPY which will turn into S_CMP/V_CMP. I think I ran into some problem when I tried to do this about 6 months ago, but I don't remember all the details.

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

https://reviews.llvm.org/D64726