llvm dev - Jun 2020 - Heads-up: Handling target-specific intrinsics in InstCombine

Hello,

this mail is to raise awareness of https://reviews.llvm.org/D81728,
which is substantial enough of a conceptual change that it should
probably at least be mentioned in llvm-dev.

InstCombine has dealt with target-specific intrinsics for a long time,
since its fix-point iteration is arguably the right place to do so. A
downside is that there's a pull to add an increasing amount of code
into lib/Transforms/InstCombine/ that really is target-specific.

What the aforementioned change does is provide a target hook via
TargetTransformInfo, and move the handling of all target intrinsics
for InstCombine purposes into the corresponding lib/Target/$foo/
directory via the hook, thus cleaning up the core of InstCombine.

Cheers,
Nicolai
-- 
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.

+1

On 6/29/20 7:29 AM, Nicolai Hähnle via llvm-dev wrote:
> Hello,
>
> this mail is to raise awareness of https://reviews.llvm.org/D81728,
> which is substantial enough of a conceptual change that it should
> probably at least be mentioned in llvm-dev.
>
> InstCombine has dealt with target-specific intrinsics for a long time,
> since its fix-point iteration is arguably the right place to do so. A
> downside is that there's a pull to add an increasing amount of code
> into lib/Transforms/InstCombine/ that really is target-specific.
>
> What the aforementioned change does is provide a target hook via
> TargetTransformInfo, and move the handling of all target intrinsics
> for InstCombine purposes into the corresponding lib/Target/$foo/
> directory via the hook, thus cleaning up the core of InstCombine.
>
> Cheers,
> Nicolai

On 6/29/20 7:29 AM, Nicolai Hähnle via llvm-dev wrote:
> Hello,
>
> this mail is to raise awareness of https://reviews.llvm.org/D81728,
> which is substantial enough of a conceptual change that it should
> probably at least be mentioned in llvm-dev.
>
> InstCombine has dealt with target-specific intrinsics for a long time,
> since its fix-point iteration is arguably the right place to do so. A
> downside is that there's a pull to add an increasing amount of code
> into lib/Transforms/InstCombine/ that really is target-specific.
>
> What the aforementioned change does is provide a target hook via
> TargetTransformInfo, and move the handling of all target intrinsics
> for InstCombine purposes into the corresponding lib/Target/$foo/
> directory via the hook, thus cleaning up the core of InstCombine.
>
> Cheers,
> Nicolai

I said this on the review, but to repeat it here, I think that this is a 
good idea. We should allow targets to customize the behavior of the 
optimizer with respect to target-specific intrinsics using code in the 
target, not code spread throughout the optimizer. We now do this kind of 
thing using TTI in many other places, and we should do it here too.

  -Hal


-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

Nicolai Hähnle via llvm-dev <llvm-dev at lists.llvm.org> writes:
> InstCombine has dealt with target-specific intrinsics for a long time,
> since its fix-point iteration is arguably the right place to do so. A
> downside is that there's a pull to add an increasing amount of code
> into lib/Transforms/InstCombine/ that really is target-specific.
>
> What the aforementioned change does is provide a target hook via
> TargetTransformInfo, and move the handling of all target intrinsics
> for InstCombine purposes into the corresponding lib/Target/$foo/
> directory via the hook, thus cleaning up the core of InstCombine.
+1.  This is great!  I hope to see more such target optimizer hooks in
the future.

                -David

I agree this is a great direction.  I have some concerns about the details of
the patch (added to the patch) but I agree this is a great direction to go.  You
might want to check out the MLIR canonicalize pass which is effectively this but
factored in a more flexible way.

-Chris
> On Jun 30, 2020, at 9:13 AM, David Greene via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Nicolai Hähnle via llvm-dev <llvm-dev at lists.llvm.org> writes:
> 
>> InstCombine has dealt with target-specific intrinsics for a long time,
>> since its fix-point iteration is arguably the right place to do so. A
>> downside is that there's a pull to add an increasing amount of code
>> into lib/Transforms/InstCombine/ that really is target-specific.
>> 
>> What the aforementioned change does is provide a target hook via
>> TargetTransformInfo, and move the handling of all target intrinsics
>> for InstCombine purposes into the corresponding lib/Target/$foo/
>> directory via the hook, thus cleaning up the core of InstCombine.
> 
> +1.  This is great!  I hope to see more such target optimizer hooks in
> the future.
> 
>                -David
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

Definite +1 on the general direction.

Philip

On 6/29/20 5:29 AM, Nicolai Hähnle via llvm-dev wrote:
> Hello,
>
> this mail is to raise awareness of https://reviews.llvm.org/D81728,
> which is substantial enough of a conceptual change that it should
> probably at least be mentioned in llvm-dev.
>
> InstCombine has dealt with target-specific intrinsics for a long time,
> since its fix-point iteration is arguably the right place to do so. A
> downside is that there's a pull to add an increasing amount of code
> into lib/Transforms/InstCombine/ that really is target-specific.
>
> What the aforementioned change does is provide a target hook via
> TargetTransformInfo, and move the handling of all target intrinsics
> for InstCombine purposes into the corresponding lib/Target/$foo/
> directory via the hook, thus cleaning up the core of InstCombine.
>
> Cheers,
> Nicolai