Daniel Sanders
2014-Sep-19 10:34 UTC
[LLVMdev] predicates vs. requirements [TableGen, X86InstrInfo.td]
> -----Original Message----- > From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] > On Behalf Of Tom Stellard > Sent: 19 September 2014 01:36 > To: Sanjay Patel > Cc: llvmdev at cs.uiuc.edu > Subject: Re: [LLVMdev] predicates vs. requirements [TableGen, > X86InstrInfo.td] > > On Thu, Sep 18, 2014 at 03:25:07PM -0600, Sanjay Patel wrote: > > I tried to add an 'OptForSize' requirement to a pattern in X86InstrSSE.td, > > but it appears to be ignored. However, the condition was detected when > > specified as a predicate. > > > > So this doesn't work: > > def : Pat<(v2f64 (X86VBroadcast (loadf64 addr:$src))), (VMOVDDUPrm > addr: > > $src)>, > > *Requires<[OptForSize**]>*; > > > > But this does: > > * let Predicates = [OptForSize] in* { > > def : Pat<(v2f64 (X86VBroadcast (loadf64 addr:$src))), (VMOVDDUPrm > addr > > :$src)>; > > } > > > > I see both forms used on some patterns like this: > > * let Predicates = [HasAVX] *in { > > def : Pat<(X86Movddup (loadv2f64 addr:$src)), > > (VMOVDDUPrm addr:$src)>, *Requires<[HasAVX]>*; > > } > > > > Is a predicate different than a requirement or is this a bug? > > > > There are existing patterns that specify 'OptForSize' with "Requires", but > > they are not behaving as I expected. > > > > Example: > > // For scalar unary operations, fold a load into the operation > > // only in OptForSize mode. It eliminates an instruction, but it also > > // eliminates a whole-register clobber (the load), so it introduces a > > // partial register update condition. > > def : Pat<(f32 (fsqrt (load addr:$src))), > > (VSQRTSSm (f32 (IMPLICIT_DEF)), addr:$src)>, > > Requires<[HasAVX, OptForSize]>; > > > > This is generated: > > vsqrtss (%rdi), %xmm0, %xmm0 > > > > regardless of whether I specify -Os or -O1 with clang. > > You might want to take a look at the Mips target, it has a > class called PredicateControl defined in Mips.td, which > handles merging predicates together into a single list. > > Maybe you could do something similar for x86. > > -TomThat'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. 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: let Predicates = [OptForSize] in { def : Pat<(v2f64 (X86VBroadcast (loadf64 addr:$src))), (VMOVDDUPrm addr:$src)>, Requires<[Foo]>, Requires<[Bar]>; } the value of Predicates in the record goes through the following values: * Undefined // Initial empty record * [] // The Pat has been applied * [Foo] // The first Requires<> has been applied. * [Bar] // The second Requires<> has been applied. * [OptForSize] // The let Predicates = ... in has been applied.
Sanjay Patel
2014-Sep-19 17:26 UTC
[LLVMdev] predicates vs. requirements [TableGen, X86InstrInfo.td]
Thanks everyone for the explanation. Can this be changed in tablegen itself or do targets rely on the current behavior? A hacky check shows that X86 is the main offender, but Sparc and Hexagon also have possible errors related to this: Target$ grep --include="*.td" -rHn "$" . | awk '/Predicates/,/\}/'| grep Requires |grep -v Additional ./Hexagon/HexagonInstrInfoV4.td:3110: Requires<[HasV4T]>; ./Hexagon/HexagonInstrInfoV4.td:3123: Requires<[HasV4T]>; ./Hexagon/HexagonInstrInfoV4.td:3135: Requires<[HasV4T]>; ./Sparc/SparcInstr64Bit.td:443: Requires<[HasHardQuad]>; ./Sparc/SparcInstr64Bit.td:457: Requires<[HasHardQuad]>; ./Sparc/SparcInstrInfo.td:1019: Requires<[HasHardQuad]>; ./Sparc/SparcInstrInfo.td:1028: Requires<[HasHardQuad]>; ./Sparc/SparcInstrInfo.td:1037: Requires<[HasHardQuad]>; ./Sparc/SparcInstrInfo.td:1088: Requires<[HasHardQuad]>; ./X86/X86InstrAVX512.td:4326: Requires<[OptForSize]>; ./X86/X86InstrAVX512.td:4331: Requires<[OptForSize]>; ./X86/X86InstrAVX512.td:4337: Requires<[OptForSize]>; ./X86/X86InstrAVX512.td:4343: Requires<[OptForSize]>; ./X86/X86InstrSSE.td:3629: (VSQRTSSr (f32 (IMPLICIT_DEF)), FR32:$src)>, Requires<[HasAVX]>; ./X86/X86InstrSSE.td:3632: Requires<[HasAVX, OptForSize]>; ./X86/X86InstrSSE.td:3634: (VSQRTSDr (f64 (IMPLICIT_DEF)), FR64:$src)>, Requires<[HasAVX]>; ./X86/X86InstrSSE.td:3637: Requires<[HasAVX, OptForSize]>; ./X86/X86InstrSSE.td:3640: (VRSQRTSSr (f32 (IMPLICIT_DEF)), FR32:$src)>, Requires<[HasAVX]>; ./X86/X86InstrSSE.td:3643: Requires<[HasAVX, OptForSize]>; ./X86/X86InstrSSE.td:3646: (VRCPSSr (f32 (IMPLICIT_DEF)), FR32:$src)>, Requires<[HasAVX]>; ./X86/X86InstrSSE.td:3649: Requires<[HasAVX, OptForSize]>; ./X86/X86InstrSSE.td:5273: (VMOVDDUPrm addr:$src)>, Requires<[HasAVX]>; ./X86/X86InstrSSE.td:5275: (VMOVDDUPrm addr:$src)>, Requires<[HasAVX]>; ./X86/X86InstrSSE.td:5277: (VMOVDDUPrm addr:$src)>, Requires<[HasAVX]>; ./X86/X86InstrSSE.td:5280: (VMOVDDUPrm addr:$src)>, Requires<[HasAVX]>; On Fri, Sep 19, 2014 at 4:34 AM, Daniel Sanders <Daniel.Sanders at imgtec.com> wrote:> > > > -----Original Message----- > > From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] > > On Behalf Of Tom Stellard > > Sent: 19 September 2014 01:36 > > To: Sanjay Patel > > Cc: llvmdev at cs.uiuc.edu > > Subject: Re: [LLVMdev] predicates vs. requirements [TableGen, > > X86InstrInfo.td] > > > > On Thu, Sep 18, 2014 at 03:25:07PM -0600, Sanjay Patel wrote: > > > I tried to add an 'OptForSize' requirement to a pattern in > X86InstrSSE.td, > > > but it appears to be ignored. However, the condition was detected when > > > specified as a predicate. > > > > > > So this doesn't work: > > > def : Pat<(v2f64 (X86VBroadcast (loadf64 addr:$src))), (VMOVDDUPrm > > addr: > > > $src)>, > > > *Requires<[OptForSize**]>*; > > > > > > But this does: > > > * let Predicates = [OptForSize] in* { > > > def : Pat<(v2f64 (X86VBroadcast (loadf64 addr:$src))), (VMOVDDUPrm > > addr > > > :$src)>; > > > } > > > > > > I see both forms used on some patterns like this: > > > * let Predicates = [HasAVX] *in { > > > def : Pat<(X86Movddup (loadv2f64 addr:$src)), > > > (VMOVDDUPrm addr:$src)>, *Requires<[HasAVX]>*; > > > } > > > > > > Is a predicate different than a requirement or is this a bug? > > > > > > There are existing patterns that specify 'OptForSize' with "Requires", > but > > > they are not behaving as I expected. > > > > > > Example: > > > // For scalar unary operations, fold a load into the operation > > > // only in OptForSize mode. It eliminates an instruction, but it also > > > // eliminates a whole-register clobber (the load), so it introduces a > > > // partial register update condition. > > > def : Pat<(f32 (fsqrt (load addr:$src))), > > > (VSQRTSSm (f32 (IMPLICIT_DEF)), addr:$src)>, > > > Requires<[HasAVX, OptForSize]>; > > > > > > This is generated: > > > vsqrtss (%rdi), %xmm0, %xmm0 > > > > > > regardless of whether I specify -Os or -O1 with clang. > > > > You might want to take a look at the Mips target, it has a > > class called PredicateControl defined in Mips.td, which > > handles merging predicates together into a single list. > > > > Maybe you could do something similar for x86. > > > > -Tom > > 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. > > 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: > let Predicates = [OptForSize] in { > def : Pat<(v2f64 (X86VBroadcast (loadf64 addr:$src))), (VMOVDDUPrm > addr:$src)>, Requires<[Foo]>, Requires<[Bar]>; > } > the value of Predicates in the record goes through the following values: > * Undefined // Initial empty record > * [] // The Pat has been applied > * [Foo] // The first Requires<> has been applied. > * [Bar] // The second Requires<> has been applied. > * [OptForSize] // The let Predicates = ... in has been applied. >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140919/58aaea07/attachment.html>
Hal Finkel
2014-Sep-19 20:33 UTC
[LLVMdev] predicates vs. requirements [TableGen, X86InstrInfo.td]
----- Original Message -----> From: "Sanjay Patel" <spatel at rotateright.com> > To: "Daniel Sanders" <Daniel.Sanders at imgtec.com> > Cc: llvmdev at cs.uiuc.edu > Sent: Friday, September 19, 2014 12:26:09 PM > Subject: Re: [LLVMdev] predicates vs. requirements [TableGen, X86InstrInfo.td] > > Thanks everyone for the explanation. Can this be changed in tablegen > itself or do targets rely on the current behavior?My preference is to update TableGen so that it is possible to append to the list (which would also help with defining register uses/defs). -Hal> > A hacky check shows that X86 is the main offender, but Sparc and > Hexagon also have possible errors related to this: > > Target$ grep --include="*.td" -rHn "$" . | awk '/Predicates/,/\}/'| > grep Requires |grep -v Additional > ./Hexagon/HexagonInstrInfoV4.td:3110: Requires<[HasV4T]>; > ./Hexagon/HexagonInstrInfoV4.td:3123: Requires<[HasV4T]>; > ./Hexagon/HexagonInstrInfoV4.td:3135: Requires<[HasV4T]>; > ./Sparc/SparcInstr64Bit.td:443: Requires<[HasHardQuad]>; > ./Sparc/SparcInstr64Bit.td:457: Requires<[HasHardQuad]>; > ./Sparc/SparcInstrInfo.td:1019: Requires<[HasHardQuad]>; > ./Sparc/SparcInstrInfo.td:1028: Requires<[HasHardQuad]>; > ./Sparc/SparcInstrInfo.td:1037: Requires<[HasHardQuad]>; > ./Sparc/SparcInstrInfo.td:1088: Requires<[HasHardQuad]>; > ./X86/X86InstrAVX512.td:4326: Requires<[OptForSize]>; > ./X86/X86InstrAVX512.td:4331: Requires<[OptForSize]>; > ./X86/X86InstrAVX512.td:4337: Requires<[OptForSize]>; > ./X86/X86InstrAVX512.td:4343: Requires<[OptForSize]>; > ./X86/X86InstrSSE.td:3629: (VSQRTSSr (f32 (IMPLICIT_DEF)), > FR32:$src)>, Requires<[HasAVX]>; > ./X86/X86InstrSSE.td:3632: Requires<[HasAVX, OptForSize]>; > ./X86/X86InstrSSE.td:3634: (VSQRTSDr (f64 (IMPLICIT_DEF)), > FR64:$src)>, Requires<[HasAVX]>; > ./X86/X86InstrSSE.td:3637: Requires<[HasAVX, OptForSize]>; > ./X86/X86InstrSSE.td:3640: (VRSQRTSSr (f32 (IMPLICIT_DEF)), > FR32:$src)>, Requires<[HasAVX]>; > ./X86/X86InstrSSE.td:3643: Requires<[HasAVX, OptForSize]>; > ./X86/X86InstrSSE.td:3646: (VRCPSSr (f32 (IMPLICIT_DEF)), > FR32:$src)>, Requires<[HasAVX]>; > ./X86/X86InstrSSE.td:3649: Requires<[HasAVX, OptForSize]>; > ./X86/X86InstrSSE.td:5273: (VMOVDDUPrm addr:$src)>, > Requires<[HasAVX]>; > ./X86/X86InstrSSE.td:5275: (VMOVDDUPrm addr:$src)>, > Requires<[HasAVX]>; > ./X86/X86InstrSSE.td:5277: (VMOVDDUPrm addr:$src)>, > Requires<[HasAVX]>; > ./X86/X86InstrSSE.td:5280: (VMOVDDUPrm addr:$src)>, > Requires<[HasAVX]>; > > > > > On Fri, Sep 19, 2014 at 4:34 AM, Daniel Sanders < > Daniel.Sanders at imgtec.com > wrote: > > > > > > -----Original Message----- > > From: llvmdev-bounces at cs.uiuc.edu [mailto: > > llvmdev-bounces at cs.uiuc.edu ] > > On Behalf Of Tom Stellard > > Sent: 19 September 2014 01:36 > > To: Sanjay Patel > > Cc: llvmdev at cs.uiuc.edu > > Subject: Re: [LLVMdev] predicates vs. requirements [TableGen, > > X86InstrInfo.td] > > > > > > On Thu, Sep 18, 2014 at 03:25:07PM -0600, Sanjay Patel wrote: > > > I tried to add an 'OptForSize' requirement to a pattern in > > > X86InstrSSE.td, > > > but it appears to be ignored. However, the condition was detected > > > when > > > specified as a predicate. > > > > > > So this doesn't work: > > > def : Pat<(v2f64 (X86VBroadcast (loadf64 addr:$src))), > > > (VMOVDDUPrm > > addr: > > > $src)>, > > > *Requires<[OptForSize**]>*; > > > > > > But this does: > > > * let Predicates = [OptForSize] in* { > > > def : Pat<(v2f64 (X86VBroadcast (loadf64 addr:$src))), > > > (VMOVDDUPrm > > addr > > > :$src)>; > > > } > > > > > > I see both forms used on some patterns like this: > > > * let Predicates = [HasAVX] *in { > > > def : Pat<(X86Movddup (loadv2f64 addr:$src)), > > > (VMOVDDUPrm addr:$src)>, *Requires<[HasAVX]>*; > > > } > > > > > > Is a predicate different than a requirement or is this a bug? > > > > > > There are existing patterns that specify 'OptForSize' with > > > "Requires", but > > > they are not behaving as I expected. > > > > > > Example: > > > // For scalar unary operations, fold a load into the operation > > > // only in OptForSize mode. It eliminates an instruction, but it > > > also > > > // eliminates a whole-register clobber (the load), so it > > > introduces a > > > // partial register update condition. > > > def : Pat<(f32 (fsqrt (load addr:$src))), > > > (VSQRTSSm (f32 (IMPLICIT_DEF)), addr:$src)>, > > > Requires<[HasAVX, OptForSize]>; > > > > > > This is generated: > > > vsqrtss (%rdi), %xmm0, %xmm0 > > > > > > regardless of whether I specify -Os or -O1 with clang. > > > > You might want to take a look at the Mips target, it has a > > class called PredicateControl defined in Mips.td, which > > handles merging predicates together into a single list. > > > > Maybe you could do something similar for x86. > > > > -Tom > > 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. > > 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: > let Predicates = [OptForSize] in { > def : Pat<(v2f64 (X86VBroadcast (loadf64 addr:$src))), (VMOVDDUPrm > addr:$src)>, Requires<[Foo]>, Requires<[Bar]>; > } > the value of Predicates in the record goes through the following > values: > * Undefined // Initial empty record > * [] // The Pat has been applied > * [Foo] // The first Requires<> has been applied. > * [Bar] // The second Requires<> has been applied. > * [OptForSize] // The let Predicates = ... in has been applied. > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-- Hal Finkel Assistant Computational Scientist Leadership Computing Facility Argonne National Laboratory