<div dir="ltr"><div>Thanks everyone for the explanation. Can this be changed in tablegen itself or do targets rely on the current behavior?<br><br></div>A hacky check shows that X86 is the main offender, but Sparc and Hexagon also have possible errors related to this:<br><br>Target$ grep --include="*.td" -rHn "$" . | awk '/Predicates/,/\}/'| grep Requires |grep -v Additional<br>./Hexagon/HexagonInstrInfoV4.td:3110: Requires<[HasV4T]>;<br>./Hexagon/HexagonInstrInfoV4.td:3123: Requires<[HasV4T]>;<br>./Hexagon/HexagonInstrInfoV4.td:3135: Requires<[HasV4T]>;<br>./Sparc/SparcInstr64Bit.td:443: Requires<[HasHardQuad]>;<br>./Sparc/SparcInstr64Bit.td:457: Requires<[HasHardQuad]>;<br>./Sparc/SparcInstrInfo.td:1019: Requires<[HasHardQuad]>;<br>./Sparc/SparcInstrInfo.td:1028: Requires<[HasHardQuad]>;<br>./Sparc/SparcInstrInfo.td:1037: Requires<[HasHardQuad]>;<br>./Sparc/SparcInstrInfo.td:1088: Requires<[HasHardQuad]>;<br>./X86/X86InstrAVX512.td:4326: Requires<[OptForSize]>;<br>./X86/X86InstrAVX512.td:4331: Requires<[OptForSize]>;<br>./X86/X86InstrAVX512.td:4337: Requires<[OptForSize]>;<br>./X86/X86InstrAVX512.td:4343: Requires<[OptForSize]>;<br>./X86/X86InstrSSE.td:3629: (VSQRTSSr (f32 (IMPLICIT_DEF)), FR32:$src)>, Requires<[HasAVX]>;<br>./X86/X86InstrSSE.td:3632: Requires<[HasAVX, OptForSize]>;<br>./X86/X86InstrSSE.td:3634: (VSQRTSDr (f64 (IMPLICIT_DEF)), FR64:$src)>, Requires<[HasAVX]>;<br>./X86/X86InstrSSE.td:3637: Requires<[HasAVX, OptForSize]>;<br>./X86/X86InstrSSE.td:3640: (VRSQRTSSr (f32 (IMPLICIT_DEF)), FR32:$src)>, Requires<[HasAVX]>;<br>./X86/X86InstrSSE.td:3643: Requires<[HasAVX, OptForSize]>;<br>./X86/X86InstrSSE.td:3646: (VRCPSSr (f32 (IMPLICIT_DEF)), FR32:$src)>, Requires<[HasAVX]>;<br>./X86/X86InstrSSE.td:3649: Requires<[HasAVX, OptForSize]>;<br>./X86/X86InstrSSE.td:5273: (VMOVDDUPrm addr:$src)>, Requires<[HasAVX]>;<br>./X86/X86InstrSSE.td:5275: (VMOVDDUPrm addr:$src)>, Requires<[HasAVX]>;<br>./X86/X86InstrSSE.td:5277: (VMOVDDUPrm addr:$src)>, Requires<[HasAVX]>;<br>./X86/X86InstrSSE.td:5280: (VMOVDDUPrm addr:$src)>, Requires<[HasAVX]>;<br><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 19, 2014 at 4:34 AM, Daniel Sanders <span dir="ltr"><<a href="mailto:Daniel.Sanders@imgtec.com" target="_blank">Daniel.Sanders@imgtec.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
> -----Original Message-----<br>
> From: <a href="mailto:llvmdev-bounces@cs.uiuc.edu">llvmdev-bounces@cs.uiuc.edu</a> [mailto:<a href="mailto:llvmdev-bounces@cs.uiuc.edu">llvmdev-bounces@cs.uiuc.edu</a>]<br>
> On Behalf Of Tom Stellard<br>
> Sent: 19 September 2014 01:36<br>
> To: Sanjay Patel<br>
> Cc: <a href="mailto:llvmdev@cs.uiuc.edu">llvmdev@cs.uiuc.edu</a><br>
> Subject: Re: [LLVMdev] predicates vs. requirements [TableGen,<br>
> X86InstrInfo.td]<br>
><br>
</span><div><div class="h5">> On Thu, Sep 18, 2014 at 03:25:07PM -0600, Sanjay Patel wrote:<br>
> > I tried to add an 'OptForSize' requirement to a pattern in X86InstrSSE.td,<br>
> > but it appears to be ignored. However, the condition was detected when<br>
> > specified as a predicate.<br>
> ><br>
> > So this doesn't work:<br>
> > def : Pat<(v2f64 (X86VBroadcast (loadf64 addr:$src))), (VMOVDDUPrm<br>
> addr:<br>
> > $src)>,<br>
> > *Requires<[OptForSize**]>*;<br>
> ><br>
> > But this does:<br>
> > * let Predicates = [OptForSize] in* {<br>
> > def : Pat<(v2f64 (X86VBroadcast (loadf64 addr:$src))), (VMOVDDUPrm<br>
> addr<br>
> > :$src)>;<br>
> > }<br>
> ><br>
> > I see both forms used on some patterns like this:<br>
> > * let Predicates = [HasAVX] *in {<br>
> > def : Pat<(X86Movddup (loadv2f64 addr:$src)),<br>
> > (VMOVDDUPrm addr:$src)>, *Requires<[HasAVX]>*;<br>
> > }<br>
> ><br>
> > Is a predicate different than a requirement or is this a bug?<br>
> ><br>
> > There are existing patterns that specify 'OptForSize' with "Requires", but<br>
> > they are not behaving as I expected.<br>
> ><br>
> > Example:<br>
> > // For scalar unary operations, fold a load into the operation<br>
> > // only in OptForSize mode. It eliminates an instruction, but it also<br>
> > // eliminates a whole-register clobber (the load), so it introduces a<br>
> > // partial register update condition.<br>
> > def : Pat<(f32 (fsqrt (load addr:$src))),<br>
> > (VSQRTSSm (f32 (IMPLICIT_DEF)), addr:$src)>,<br>
> > Requires<[HasAVX, OptForSize]>;<br>
> ><br>
> > This is generated:<br>
> > vsqrtss (%rdi), %xmm0, %xmm0<br>
> ><br>
> > regardless of whether I specify -Os or -O1 with clang.<br>
><br>
> You might want to take a look at the Mips target, it has a<br>
> class called PredicateControl defined in Mips.td, which<br>
> handles merging predicates together into a single list.<br>
><br>
> Maybe you could do something similar for x86.<br>
><br>
> -Tom<br>
<br>
</div></div>That's right, it's solving the same problem. I added it after I found several defs that attempted to add predicates but accidentally removed some of the existing ones at the same time.<br>
<br>
The root of the problem is the way tablegen processes the declarations and assignments. tablegen creates an empty record and applies the subclasses in left-to-right order. Then it applies any 'let Foo = Bar in' statements. So given this code:<br>
<span class=""> let Predicates = [OptForSize] in {<br>
</span> def : Pat<(v2f64 (X86VBroadcast (loadf64 addr:$src))), (VMOVDDUPrm addr:$src)>, Requires<[Foo]>, Requires<[Bar]>;<br>
}<br>
the value of Predicates in the record goes through the following values:<br>
* Undefined // Initial empty record<br>
* [] // The Pat has been applied<br>
* [Foo] // The first Requires<> has been applied.<br>
* [Bar] // The second Requires<> has been applied.<br>
* [OptForSize] // The let Predicates = ... in has been applied.<br>
</blockquote></div><br></div>