[PATCH] Make InstCombine aware of TargetTransformInfo when optimize extension

Hal Finkel hfinkel at anl.gov
Thu Jul 2 05:33:30 PDT 2015


----- Original Message -----
> From: "Jingyue Wu" <jingyue at google.com>
> To: "David Majnemer" <david.majnemer at gmail.com>
> Cc: reviews+D10750+public+f73185f8d9b270d8 at reviews.llvm.org, "Justin Holewinski" <jholewinski at nvidia.com>,
> llvm-commits at cs.uiuc.edu, "thomas stellard" <thomas.stellard at amd.com>
> Sent: Wednesday, July 1, 2015 4:09:59 PM
> Subject: Re: [PATCH] Make InstCombine aware of TargetTransformInfo when	optimize extension
> 
> 
> 
> Hi David,
> 
> 
> The thread you cited is talking about DAGTypeLegalizer::isTypeLegal
> which looks separate from DataLayout::isLegalInteger we've been
> talking about here. Maybe LLVM assumes they should be consistent?
> 
> 
> I see two approaches here.
> 
> 
> 1. Making 64-bit illegal in NVPTX's data layout. This will solve the
> problem this patch originates from, because ShouldChangeType already
> disallows conversion to data-layout-illegal types. I also suspect
> this won't break the codegen, because DAGTypeLegalizer still
> considers 64-bit a legal type and is able to emit 64-bit PTX
> instructions such as add.s64. However, this will cause performance
> issues in LSR (which is being discussed in Bjarke's thread
> http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-June/087234.html )
> and maybe other passes. Currently, LSR only promotes variables with
> legal bitwidths to indvars. Pointers in NVPTX64 are 64-bit so LSR,
> so LSR won't consider them at all. Therefore, if we make 64-bit
> data-layout-illegal, we will have to fix some callers of
> DataLayout::isLegalInteger to handle non-native integer/pointer
> types (using TTI's instruction cost model e.g.) to get good
> performance.

In the name of keeping proper cross-references between threads, as I commented on the other thread (http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-July/087497.html), I believe that this option is the right one. You can keep i64 legal in the backend but not have it be listed as legal in DataLayout (because it really is not legal on the hardware, our backend just happens to not be directly responsible for the i64 -> i32 legalization). We should probably isolate discussion of this option to the other thread for now, because it is on llvmdev, and has grown to discuss the various implications of this path.

 -Hal

> 
> 
> 2. (the approach taken in this patch) Keeping 64-bit legal in NVPTX's
> data layout, and let the callers of isLegalInteger (InstCombineCasts
> in this case) be aware of cost difference in legal types.
> 
> 
> I don't see a clear winner. Should 64-bit be legal or illegal in
> NVPTX's data layout? NVPTX emits PTX ISA in which 64-bit is native,
> but 64-bit in not native in the more low-level machine code. Any
> thoughts?
> 
> 
> Jingyue
> 
> 
> 
> 
> On Wed, Jul 1, 2015 at 11:44 AM, David Majnemer <
> david.majnemer at gmail.com > wrote:
> 
> 
> 
> 
> 
> 
> 
> On Tue, Jun 30, 2015 at 9:34 AM, Justin Holewinski <
> jholewinski at nvidia.com > wrote:
> 
> 
> 
> > On Jun 26, 2015, at 4:20 AM, Chandler Carruth < chandlerc at gmail.com
> > > wrote:
> > 
> > This doesn't seem like the right approach at all.
> > 
> > I don't think we want to use cost heuristics to dictate the
> > canonical form in the IR. It makes the canonicalization *much*
> > more complex and hard to manage.
> 
> I agree. I would prefer not to mix the concepts of legality and
> performance here.
> 
> > 
> > I think that NVPTX should either change the datalayout to make i64
> > illegal (and that seems reasonable for instcombine to respect, but
> > that would for example mean it would be an *absolute* decision,
> > not a cost decision) or you'll need to do a late transform to
> > narrow the operations as much as possible.
> 
> I don’t see how we could remove i64 as a legal type in NVPTX. Our
> pointer size is 64-bit, so we need a container for pointers.
> 
> 
> 
> I think other folks are doing (or would like to be doing) this sort
> of thing:
> http://lists.cs.uiuc.edu/pipermail/llvmdev/2015-June/087248.html
> 
> 
> 
> > 
> > 
> > http://reviews.llvm.org/D10750
> > 
> > EMAIL PREFERENCES
> > http://reviews.llvm.org/settings/panel/emailpreferences/
> > 
> > 
> 
> 
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s)
> and may contain
> confidential information. Any unauthorized review, use, disclosure or
> distribution
> is prohibited. If you are not the intended recipient, please contact
> the sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory




More information about the llvm-commits mailing list