[PATCH] D86200: [GlobalISel] Add setting of pointer flags to ArgInfo

[Gabriel Hjort Åkerlund via Phabricator via llvm-commits]

ehjogab abandoned this revision.
ehjogab added a comment.

In D86200#2226354 <https://reviews.llvm.org/D86200#2226354>, @arsenm wrote:

> In D86200#2226337 <https://reviews.llvm.org/D86200#2226337>, @ehjogab wrote:
>
>> In D86200#2226077 <https://reviews.llvm.org/D86200#2226077>, @arsenm wrote:
>>
>>> I'm curious why you need this. These fields are mostly a hack for SelectionDAG?
>>
>> In our target argument pointers are converted into MVT::iPTRs, but in light of your comment that might be a bug in our backend. Could you comment on how, and if, MVT::iPTR are suppose to be used in GlobalISel, or are they only intended for SelectionISel?
>
> Ideally MVT wouldn't be used at all, and we're just using the existing SelectionDAG calling convention lowering with its ties to MVT as a crutch. LLT naturally preserves that the value is a pointer with address space and size

I see. I compared the implementation we have against that of AArch64, and I notice a small difference in when splitting the argument: Both invoke ComputeValueVTs(), but if the number of splits is 1 (meaning there is no splitting) then AArch64 takes the single value produced by ComputeValueVTs() whereas our backend simply uses what was input to ComputeValueVTs. This means that, if the value was a pointer, our backend retains the pointer value whereas AArch64 uses the converted value, which is an integer. I actually think that the correct way is to use the converted value instead of retaining the pointer value, which means that this patch is redundant and hence to be abandoned.

Thanks for the feedback!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86200