llvm dev - Dec 2021 - [RFC] : LLVM IR should allow bitcast between address spaces with the same size

On Mon, 29 Nov 2021 at 18:15, Matt Arsenault <arsenm2 at gmail.com> wrote:
> This is true, but this isn’t directly related to this change. If the
> target does not have no-op conversions between a given pair of address
> spaces, the original code was broken to begin with. This change does not
> allow introducing new illegal address space conversions that were not
> already present in the original program. It’s garbage in, garbage out. If
> the target can’t really reinterpret these pointers, it would be UB on
> access.
>
Ah, I see.

So IIUC, adding the casts in the first place would have to know about
target address spaces, but this specific change only allows eliding the
inttoptr cast in the specific case where the address spaces have the same
data layout?

Assuming the initial cast was legal to begin with, this looks sensible to
me.

I was worried this would allow (encourage?) front-ends and other passes to
add an address space cast where none (could have) existed in the first
place.

Sorry for the noise.

--renato
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211130/f054c628/attachment.html>

> On Nov 30, 2021, at 04:51, Renato Golin <rengolin at gmail.com>
wrote:
> 
> On Mon, 29 Nov 2021 at 18:15, Matt Arsenault <arsenm2 at gmail.com
<mailto:arsenm2 at gmail.com>> wrote:
> This is true, but this isn’t directly related to this change. If the target
does not have no-op conversions between a given pair of address spaces, the
original code was broken to begin with. This change does not allow introducing
new illegal address space conversions that were not already present in the
original program. It’s garbage in, garbage out. If the target can’t really
reinterpret these pointers, it would be UB on access.
> 
> Ah, I see.
> 
> So IIUC, adding the casts in the first place would have to know about
target address spaces, but this specific change only allows eliding the inttoptr
cast in the specific case where the address spaces have the same data layout?
> 
> Assuming the initial cast was legal to begin with, this looks sensible to
me.
The target’s notion of address space casts doesn’t apply here. The user decided
to reinterpret the pointer in a different address space, which may or may not be
valid to use on the target.
> 
> I was worried this would allow (encourage?) front-ends and other passes to
add an address space cast where none (could have) existed in the first place.
No, we’re converting from an in-memory representation of reinterpreting bits to
one in values. Frontends still need to have some contextual knowledge of what
the language and target expects out of address spaces.

-Matt
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211130/4b0c9913/attachment.html>

[AMD Official Use Only]

Re-iterating the proposed change and clarifying the concerns raised :

Proposed change:
- Allow no-op bitcast between pointers of same size but from different
addrspaces.

Clarifying the concerns:
- No-op reinterprent of bits between addrspaces is already done in the IR like
introducing ptrtoint/inttoptr(ex. GVN).
  Main motivation for this change is to give the IR a way to represent a cast we
know is a no-op without resorting to ptrtoint/inttoptr.

- With this change we do not want to introduce new illegal address space
conversions that were not already present in the original program.
  The original code was not performing an address space cast, it was doing type
punning and reinterpreting some bits as a different address space.
  It was broken to begin with if the target didn't allow no-op conversions
between given pair of address spaces. We are not intending to change that.
  Since the IR transform like in GVN case, did a no-op reinterpretation of bits
between addrspaces, We are just trying to optimise it by using a no-op bitcast.

- Since the original IR already did this reinterpret of bits without querying
the target, the change we propose will also not query the target.
  This pointer-reinterpret-to-cast transformation is and should be done
unconditionally. Querying TTI to check for no-op would introduce target
dependence on Core IR instruction semantics.
  We used DataLayout to match the pointer sizes. This check is sufficient to
allow the no-op bitcast.

- Also, there is no target specific pattern to search for here. Irrespective of
the target, we need to introduce new IR that represents the same no-op
reinterpret of bits between addrspaces.
  All the targets are intended to benefit from this change.

Regards,
Chaitanya
________________________________
From: Matt Arsenault <whatmannerofburgeristhis at gmail.com> on behalf of
Matt Arsenault <arsenm2 at gmail.com>
Sent: 01 December 2021 05:57
To: Renato Golin <rengolin at gmail.com>
Cc: Sankisa, Krishna (Chaitanya) <Krishna.Sankisa at amd.com>; llvm-dev at
lists.llvm.org <llvm-dev at lists.llvm.org>
Subject: Re: [llvm-dev] [RFC] : LLVM IR should allow bitcast between address
spaces with the same size

[CAUTION: External Email]


On Nov 30, 2021, at 04:51, Renato Golin <rengolin at
gmail.com<mailto:rengolin at gmail.com>> wrote:

On Mon, 29 Nov 2021 at 18:15, Matt Arsenault <arsenm2 at
gmail.com<mailto:arsenm2 at gmail.com>> wrote:
This is true, but this isn’t directly related to this change. If the target does
not have no-op conversions between a given pair of address spaces, the original
code was broken to begin with. This change does not allow introducing new
illegal address space conversions that were not already present in the original
program. It’s garbage in, garbage out. If the target can’t really reinterpret
these pointers, it would be UB on access.

Ah, I see.

So IIUC, adding the casts in the first place would have to know about target
address spaces, but this specific change only allows eliding the inttoptr cast
in the specific case where the address spaces have the same data layout?

Assuming the initial cast was legal to begin with, this looks sensible to me.
The target’s notion of address space casts doesn’t apply here. The user decided
to reinterpret the pointer in a different address space, which may or may not be
valid to use on the target.


I was worried this would allow (encourage?) front-ends and other passes to add
an address space cast where none (could have) existed in the first place.

No, we’re converting from an in-memory representation of reinterpreting bits to
one in values. Frontends still need to have some contextual knowledge of what
the language and target expects out of address spaces.

-Matt
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211207/d239ff1b/attachment.html>