[PATCH] D109032: [AMDGPU][NFC] Alter ComplexPattern types to be consistent with their uses

Mon Nov 1 16:06:45 PDT 2021

jrtc27 added a comment.

In D109032#3101169 <https://reviews.llvm.org/D109032#3101169>, @arsenm wrote:

> In D109032#3020826 <https://reviews.llvm.org/D109032#3020826>, @jrtc27 wrote:
>
>>>> The MUBUFAddr64 case isn't that interesting, since you're changing one form of "i64" to another form of "i64". I'm much more puzzled about how this is not blowing up on the cases that do use 32 bit pointers (e.g. all the DS* patterns)
>>>
>>> Those examples may not be what you intend them to be, but I can promise you that if you go read AMDGPUGenDAGISel.inc yourself you will find that every single one of them is currently matching an iPTR. For example:
>>>
>>>   // Src: (ld:{ *:[i32] } (ScratchOffset:{ *:[iPTR] } VGPR_32:{ *:[i32] }:$vaddr, i16:{ *:[i16] }:$offset))<<P:Predicate_unindexedload>><<P:Predicate_extload>><<P:Predicate_extloadi8_private>> - Complexity = 22
>>>   // Dst: (SCRATCH_LOAD_UBYTE:{ *:[i32] } ?:{ *:[i32] }:$vaddr, ?:{ *:[i16] }:$offset)
>>>
>>> is what I see *without* any of my patches, so I'm just preserving that. Things blow up with failures to select, or poor codegen that regresses tests, when these instead infer i32. If you want to see for yourself, apply D109035 <https://reviews.llvm.org/D109035>, note that the TableGen output changes to honour the existing intended types and that many AMDGPU tests fail.
>>
>> Ping regarding this explanation?
>
> This is more of another datapoint that nothing with iPTR makes any sense, I don't think this really explains it.

Oh I know, I'm cursed with having to bodge iPTR downstream even more than normal because we're SelectionDAG-based (and I don't see RISC-V switching to GISel any time soon)...

Thanks for the review

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109032