llvm dev - Jan 2018 - GEP transformation by InstCombiner

Hi all,

       I'm working on an out-of-tree target and encountered the following
problem:

       InstCombiner "normalizes" GEPs and extends Index operand to the
Pointer width.
       It works fine if you can convert pointer to integer for address
calculation and I assume that all registered targets do this.

       The target I'm working on has very restricted ISA for the pointer
calculation:
       ptr + int,   ptr - int,    ptr - ptr    and   ptr-compare

       I have full arithmetic set for 32-bit integers, but the Ptr is wider.
Extending index to the Ptr width requires full arithmetic support for pointers.
       But, actually, it does not come from C-sources (casting Ptr to int means
truncation).

       I'd like to add TTI (TargetTransformInfo) to InstCombiner in order to
configure the width of GEP indices.
       The current default behavior will be preserved.
       What do you think?

       Thanks.

-        Elena



---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180115/c915e03b/attachment.html>

On 01/15/2018 12:21 PM, Demikhovsky, Elena wrote:
> Hi all,
>  
> I’m working on an out-of-tree target and encountered the following
> problem:
>  
> InstCombiner “normalizes” GEPs and extends Index operand to the
> Pointer width.
> It works fine if you can convert pointer to integer for address
> calculation and I assume that all registered targets do this.
>  
> The target I’m working on has very restricted ISA for the pointer
> calculation:
> ptr + int,   ptr - int,    ptr - ptr    and   ptr-compare
>  
> I have full arithmetic set for 32-bit integers, but the Ptr is wider.
> Extending index to the Ptr width requires full arithmetic support for
> pointers.
> But, actually, it does not come from C-sources (casting Ptr to int
> means truncation).
>  
> I’d like to add TTI (TargetTransformInfo) to InstCombiner in order to
> configure the width of GEP indices.
> The current default behavior will be preserved.
> What do you think?
Given that this affects the canonical form of the IR, based on our
current practice, it should go in DataLayout (not in TTI). InstCombine
should probably know how to do the right thing for the IR even if the
particular target is not compiled in.

 -Hal
>  
> Thanks.
>  
>
>   * */Elena/*
>
>  
>  
>  
>
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
>
-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180115/3a823830/attachment.html>

I tried to retrieve anything from DataLayout. It contains pointer size, but how
can I conclude that the GEP index can't be widened?

-           Elena

From: Hal Finkel [mailto:hfinkel at anl.gov]
Sent: Monday, January 15, 2018 20:34
To: Demikhovsky, Elena <elena.demikhovsky at intel.com>; llvm-dev at
lists.llvm.org; Sanjay Patel (spatel at rotateright.com) <spatel at
rotateright.com>; Chandler Carruth (chandlerc at gmail.com) <chandlerc at
gmail.com>; Quentin Colombet (qcolombet at apple.com) <qcolombet at
apple.com>; Craig Topper (craig.topper at gmail.com) <craig.topper at
gmail.com>
Cc: Breger, Igor <igor.breger at intel.com>
Subject: Re: GEP transformation by InstCombiner


On 01/15/2018 12:21 PM, Demikhovsky, Elena wrote:
Hi all,

I'm working on an out-of-tree target and encountered the following problem:

InstCombiner "normalizes" GEPs and extends Index operand to the
Pointer width.
It works fine if you can convert pointer to integer for address calculation and
I assume that all registered targets do this.

The target I'm working on has very restricted ISA for the pointer
calculation:
ptr + int,   ptr - int,    ptr - ptr    and   ptr-compare

I have full arithmetic set for 32-bit integers, but the Ptr is wider. Extending
index to the Ptr width requires full arithmetic support for pointers.
But, actually, it does not come from C-sources (casting Ptr to int means
truncation).

I'd like to add TTI (TargetTransformInfo) to InstCombiner in order to
configure the width of GEP indices.
The current default behavior will be preserved.
What do you think?

Given that this affects the canonical form of the IR, based on our current
practice, it should go in DataLayout (not in TTI). InstCombine should probably
know how to do the right thing for the IR even if the particular target is not
compiled in.

 -Hal



Thanks.

*         Elena




---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.



--

Hal Finkel

Lead, Compiler Technology and Programming Languages

Leadership Computing Facility

Argonne National Laboratory
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180115/5cc8e7ce/attachment.html>

> On 15 Jan 2018, at 18:21, Demikhovsky, Elena via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Hi all,
>  
> I’m working on an out-of-tree target and encountered the following problem:
>  
> InstCombiner “normalizes” GEPs and extends Index operand to the Pointer
width.
> It works fine if you can convert pointer to integer for address calculation
and I assume that all registered targets do this.
>  
> The target I’m working on has very restricted ISA for the pointer
calculation:
> ptr + int,   ptr - int,    ptr - ptr    and   ptr-compare
This sounds very familiar - that’s exactly the set of operations that CHERI
supports.

We have a set of out-of-tree patches that extend the data layout to understand
that there’s a difference between the size and the range of a pointer (e.g. a
128-bit pointer can store 64-bit addresses + metadata) and fixes for all of the
optimisers that we’ve found, and SelectionDAG.  We add explicit PTRADD, INTTOPTR
and PTRTOINT nodes in SelectionDAG for architectures where pointer+integer is
distinct from integer arithmetic and where inttoptr / ptrtoint are not bitcasts.
> I have full arithmetic set for 32-bit integers, but the Ptr is wider.
Extending index to the Ptr width requires full arithmetic support for pointers.
> But, actually, it does not come from C-sources (casting Ptr to int means
truncation).
We also have clang patches that ensure that these operations do something
meaningful.
> I’d like to add TTI (TargetTransformInfo) to InstCombiner in order to
configure the width of GEP indices.
> The current default behavior will be preserved.
> What do you think?
We solved this by adding an f qualifier to the p attribute in our DataLayout to
describe pointers that are not integers and use address space 0 to define the
range.  This isn’t quite ideal, and we should probably explicitly add the range
to pointers that are wider than integers.

Note that InstCombine is not the only place that tries to insert pointer-width
GEPs in the optimisation pipeline.  I think that we’ve fixed all of them, but I
can’t be entirely sure.

We haven’t upstreamed this, because no in-tree target needs them, but I’d be
happy to try to extract the relevant parts and put them up for review if this is
generally useful.

David

> Note that InstCombine is not the only place that tries to insert
pointer-width GEPs in the optimization pipeline.  I think that we’ve fixed all
of them, but I can’t be entirely sure.
I'm going to upload a patch, but I'm fixing only the places that are
covered by our test system. I'll add you as a reviewer and you are welcome
to help me with fixing them all.
> We haven’t upstreamed this, because no in-tree target needs them, but I’d
be happy to try to extract the relevant parts and put them up for review if this
is generally useful.
We also have an internal diff, but we are trying to upstream everything that has
a common sense.


-  Elena

-----Original Message-----
From: David Chisnall [mailto:dc552 at hermes.cam.ac.uk] On Behalf Of David
Chisnall
Sent: Tuesday, January 16, 2018 12:47
To: Demikhovsky, Elena <elena.demikhovsky at intel.com>
Cc: llvm-dev at lists.llvm.org; Sanjay Patel (spatel at rotateright.com)
<spatel at rotateright.com>; Chandler Carruth (chandlerc at gmail.com)
<chandlerc at gmail.com>; Hal Finkel (hfinkel at anl.gov) <hfinkel at
anl.gov>; Quentin Colombet <qcolombet at apple.com>; Craig Topper
(craig.topper at gmail.com) <craig.topper at gmail.com>; Breger, Igor
<igor.breger at intel.com>
Subject: Re: [llvm-dev] GEP transformation by InstCombiner

> On 15 Jan 2018, at 18:21, Demikhovsky, Elena via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Hi all,
>  
> I’m working on an out-of-tree target and encountered the following problem:
>  
> InstCombiner “normalizes” GEPs and extends Index operand to the Pointer
width.
> It works fine if you can convert pointer to integer for address calculation
and I assume that all registered targets do this.
>  
> The target I’m working on has very restricted ISA for the pointer
calculation:
> ptr + int,   ptr - int,    ptr - ptr    and   ptr-compare
This sounds very familiar - that’s exactly the set of operations that CHERI
supports.

We have a set of out-of-tree patches that extend the data layout to understand
that there’s a difference between the size and the range of a pointer (e.g. a
128-bit pointer can store 64-bit addresses + metadata) and fixes for all of the
optimisers that we’ve found, and SelectionDAG.  We add explicit PTRADD, INTTOPTR
and PTRTOINT nodes in SelectionDAG for architectures where pointer+integer is
distinct from integer arithmetic and where inttoptr / ptrtoint are not bitcasts.
> I have full arithmetic set for 32-bit integers, but the Ptr is wider.
Extending index to the Ptr width requires full arithmetic support for pointers.
> But, actually, it does not come from C-sources (casting Ptr to int means
truncation).
We also have clang patches that ensure that these operations do something
meaningful.
> I’d like to add TTI (TargetTransformInfo) to InstCombiner in order to
configure the width of GEP indices.
> The current default behavior will be preserved.
> What do you think?
We solved this by adding an f qualifier to the p attribute in our DataLayout to
describe pointers that are not integers and use address space 0 to define the
range.  This isn’t quite ideal, and we should probably explicitly add the range
to pointers that are wider than integers.

Note that InstCombine is not the only place that tries to insert pointer-width
GEPs in the optimisation pipeline.  I think that we’ve fixed all of them, but I
can’t be entirely sure.

We haven’t upstreamed this, because no in-tree target needs them, but I’d be
happy to try to extract the relevant parts and put them up for review if this is
generally useful.

David

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.