<div dir="ltr">BTW, do you guys have a better name for this pass? SeparateConstOffsetFromGEP sounds too long. <div><br></div><div>Jingyue</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Apr 24, 2014 at 11:30 AM, Eli Bendersky <span dir="ltr"><<a href="mailto:eliben@google.com" target="_blank">eliben@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div class="h5">On Thu, Apr 24, 2014 at 11:28 AM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br>


<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>----- Original Message -----<br>
> From: "Eli Bendersky" <<a href="mailto:eliben@google.com" target="_blank">eliben@google.com</a>><br>
</div><div>> To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>><br>
> Cc: <a href="mailto:reviews%2BD3462%2Bpublic%2Bcbb08f1e0c1f69e1@reviews.llvm.org" target="_blank">reviews+D3462+public+cbb08f1e0c1f69e1@reviews.llvm.org</a>, <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>, "Jingyue Wu"<br>



> <<a href="mailto:jingyue@google.com" target="_blank">jingyue@google.com</a>>, "justin holewinski" <<a href="mailto:justin.holewinski@gmail.com" target="_blank">justin.holewinski@gmail.com</a>><br>

> Sent: Thursday, April 24, 2014 1:23:34 PM<br>
> Subject: Re: [PATCH] CSE on GEP indices<br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
</div><div><div>> On Thu, Apr 24, 2014 at 11:13 AM, Hal Finkel < <a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a> ><br>
> wrote:<br>
><br>
><br>
><br>
> ----- Original Message -----<br>
> > From: "Eli Bendersky" < <a href="mailto:eliben@google.com" target="_blank">eliben@google.com</a> ><br>
> > To: <a href="mailto:jingyue@google.com" target="_blank">jingyue@google.com</a> , <a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a> , <a href="mailto:eliben@google.com" target="_blank">eliben@google.com</a> ,<br>

> > "justin holewinski" < <a href="mailto:justin.holewinski@gmail.com" target="_blank">justin.holewinski@gmail.com</a> ><br>
> > Cc: <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
> > Sent: Thursday, April 24, 2014 1:06:05 PM<br>
> > Subject: Re: [PATCH] CSE on GEP indices<br>
> ><br>
> > ================<br>
> > Comment at: lib/Target/NVPTX/NVPTXTargetMachine.cpp:150<br>
> > @@ -149,5 +149,3 @@<br>
> > addPass(createNVPTXFavorNonGenericAddrSpacesPass());<br>
> > - // The FavorNonGenericAddrSpaces pass may remove instructions and<br>
> > leave some<br>
> > - // values unused. Therefore, we run a DCE pass right afterwards.<br>
> > We could<br>
> > - // remove unused values in an ad-hoc manner, but it requires<br>
> > manual work and<br>
> > - // might be error-prone.<br>
> > + addPass(createSeparateConstOffsetFromGEPPass());<br>
> > + // The SeparateConstOffsetFromGEP pass creates variadic bases<br>
> > that<br>
> > can be used<br>
> > ----------------<br>
> > Since this pass is not target independent, I'm not sure this is the<br>
> > right place to put it. It makes more sense to add it as part of a<br>
> > custom compiler optimization pipeline, or maybe even in opt itself?<br>
><br>
> Why do you say this? As I see it, this pass is essentially LSR for<br>
> the unrolled portion of multidimensional loop nests. And I asked<br>
> them to add the TTI checks and move it because it is target<br>
> independent. Because of the complex addressing modes available on<br>
> x86, I'm not sure it will help there, but I suspect it will prove<br>
> useful for several other targets (PPC, for example).<br>
><br>
> However, like LSR, it is not a canonicalization transformation, so it<br>
> belongs, along with LSR, as a backend IR pass.<br>
><br>
><br>
><br>
> Fair enough. I just wanted to avoid code duplication among different<br>
> passes, because NVPTX has its own addIRPasses. Another backend<br>
> wanting to use this (PPC, for example) would need to duplicate this<br>
> logic. Maybe it belongs in TargetPassConfig::addIRPasses() then ?<br>
<br>
</div></div>I'm not sure. We have some passes that interested targets are expected to add on their own (if conversion, for example). I don't have a strong opinion here, but I'm inclined to let the targets add the passes for now and then later, if we see a fairly-universal ordering, we can add things into TargetPassConfig with the necessary callbacks.</blockquote>


<div><br></div></div></div><div>OK, that sounds reasonable to me.</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>Eli </div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div>
<br></div><div><br></div><div><br></div><div><br></div>

</font></span></div></div></div>
</blockquote></div><br></div>