[LLVMdev] predicates vs. requirements [TableGen, X86InstrInfo.td]

[Sanjay Patel]

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.

[Hal Finkel]

----- Original Message -----
> From: "Sanjay Patel" <spa...@rotateright.com>
> To: llv...@cs.uiuc.edu
> Sent: Thursday, September 18, 2014 4:25:07 PM
> Subject: [LLVMdev] predicates vs. requirements [TableGen, X86InstrInfo.td]
>
>
>
>
> 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.

Without looking at this in detail, I've seen similar behavior before, so I'll guess: When you use Requires with OptForSize, it will add OptForSize to the Predicates list, but it will not do so if you explicitly set the Predicates list (with a let clause). So if there are any other predicates in effect, the Requires will be ignored. However, if you explicitly set Predicates (with the let clause) you're erasing any other predicates that may have been in effect from an outer block.

-Hal

>
> 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.
>

> _______________________________________________
> LLVM Developers mailing list
> LLV...@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
_______________________________________________
LLVM Developers mailing list
LLV...@cs.uiuc.edu http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

[Tom Stellard]

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

[Daniel Sanders]

> -----Original Message-----
> From: llvmdev...@cs.uiuc.edu [mailto:llvmdev...@cs.uiuc.edu]
> On Behalf Of Tom Stellard
> Sent: 19 September 2014 01:36
> To: Sanjay Patel
> Cc: llv...@cs.uiuc.edu
> Subject: Re: [LLVMdev] predicates vs. requirements [TableGen,
> X86InstrInfo.td]
>

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.

[Sanjay Patel]

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]>;

[Hal Finkel]

----- Original Message -----
> From: "Sanjay Patel" <spa...@rotateright.com>

> To: "Daniel Sanders" <Daniel....@imgtec.com>
> Cc: llv...@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

> _______________________________________________
> LLVM Developers mailing list
> LLV...@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

[Daniel Sanders]

I'd prefer that too and I had some patches back in April that attempted to do that. It was easy enough to support these cases but I found that I had broken multiclasses in the process. Unfortunately, I never found a solution to the multiclass problems and had to fall back on the PredicateControl solution.

http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20140421/214527.html is the thread I mentioned it in but there's little useful information there.