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

Tue Aug 31 17:22:44 PDT 2021

jrtc27 added a comment.

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

> I'm not sure where this is going without the context of the other patch.
>
> I think iPTR is a flawed concept that doesn't work without the address space. AMDGPU does not want the type changing magic of iPTR when there are mixed and constant pointer sizes that never change, so the fixed types are more logical.

I agree with you in a sense, it causes pain for us downstream to not have address spaces here (so we just add a new type for our other pointer representation downstream, and up until now it's been hacked together, but I'm trying to rewrite it to do things properly, and adding a new possible pointer type everywhere beyond iPTR causes a lot of believed-ambiguous-but-not patterns without better type inference in TableGen). However, if you look at the generated .inc files, you will see that all the cases where these are used TableGen has already inferred that iPTR is in use, and if I don't make this change but have my subsequent TableGen patch applied to actually propagate the types as they should have been from the start then the AMDGPU backend becomes very crash-prone with failures to select all over the place as it seems it really is expecting iPTR not i64 for various things. The changes here have zero effect on the generated files without my TableGen patch, and ensure that even with my TableGen patch the output remains unchanged (otherwise they go from iPTR to i32/i64), so this doesn't change behaviour, it just makes the code more accurately state what is actually currently going on.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109032