Random question about the x86 backend (and backends in general I suppose)

Chandler_Carruth · December 30, 2013, 7:56pm

Having worked with a few people to better understand the tablegen descriptions of instructions and patterns in LLVM’s backend and looking at x86’s pretty heavily, I have some questions:

Are there instruction definition flags that are really just “when needed”? I’m thinking of things like “mayLoad” which is really alarmingly missing from a bunch of instructions in x86 which load. Is this OK? Is this a bug, or just suboptimal?
Are all of the flags in Target.td (lines 356 - 381) the “recommended” set? Are any of them no longer really important to mark?
It would be really nice to keep track of the flags for instructions which are added (or removed) from the “recommend set”, and whether or not backends have had their instruction definitions audited to be up-to-date here. Essentially, I wonder if it would be useful to keep PRs open with a nice list of low-hanging maintenance tasks on the instruction definition tables for people interested.
If I’m reading the td files and I see things that are obviously wrong (for example, an instruction which loads but is not marked as such, or instructions which should be marked as (potentially) trivially rematerializable), should I just fix them and submit those fixes? Can I even write test cases for most of these? Clearly, what I consider “obviously wrong” will be informed by answers to #1 and #2 above.

topperc · December 30, 2013, 8:29pm

I can’t speak directly to the questions themselves, but I’ll ask a couple back. When you say that some instructions are missing mayLoad, do these instructions have patterns? Tablegen can infer mayLoad/mayStores/hasSideEffects from patterns so it doesn’t always need to be listed explicitly in the td files.

Instructions without patterns are marked hasSideEffects=1 which is more restrictive than mayLoad/mayStore.

Finkel_Hal_J · December 31, 2013, 12:17am

From: "Craig Topper" <craig.topper@gmail.com>
To: "Chandler Carruth" <chandlerc@google.com>
Cc: "LLVM Developers Mailing List" <llvmdev@cs.uiuc.edu>
Sent: Monday, December 30, 2013 2:29:50 PM
Subject: Re: [LLVMdev] Random question about the x86 backend (and backends in general I suppose)

I can't speak directly to the questions themselves, but I'll ask a
couple back. When you say that some instructions are missing
mayLoad, do these instructions have patterns? Tablegen can infer
mayLoad/mayStores/hasSideEffects from patterns so it doesn't always
need to be listed explicitly in the td files.

Having recently audited these flags in the PowerPC backend, I highly recommend looking at these from the *GenInstrInfo.inc file directly. I find this much easier. In theory, we'd like to move away from the pattern-based flag inference. Once a target is free of dependence on the inference rules, it can set bit guessInstructionProperties = 0; to turn them off completely (see class InstrInfo in Target.td).

-Hal

Chris_Lattner · December 31, 2013, 4:40am

In MHO, we should try to avoid redundancy as much as possible. The only reason to have these flags is when instructions don't have patterns. We should extend the tblgen pattern lexicon to make it possible to write patterns in more cases. I think it's embarrassing that we still can't write a pattern for a move instruction :-/

-Chris

Evan_Cheng1 · January 7, 2014, 6:40pm

From: "Craig Topper" <craig.topper@gmail.com>
To: "Chandler Carruth" <chandlerc@google.com>
Cc: "LLVM Developers Mailing List" <llvmdev@cs.uiuc.edu>
Sent: Monday, December 30, 2013 2:29:50 PM
Subject: Re: [LLVMdev] Random question about the x86 backend (and backends in general I suppose)

I can't speak directly to the questions themselves, but I'll ask a
couple back. When you say that some instructions are missing
mayLoad, do these instructions have patterns? Tablegen can infer
mayLoad/mayStores/hasSideEffects from patterns so it doesn't always
need to be listed explicitly in the td files.

Having recently audited these flags in the PowerPC backend, I highly recommend looking at these from the *GenInstrInfo.inc file directly. I find this much easier. In theory, we'd like to move away from the pattern-based flag inference. Once a target is free of dependence on the inference rules, it can set bit guessInstructionProperties = 0; to turn them off completely (see class InstrInfo in Target.td).

In MHO, we should try to avoid redundancy as much as possible. The only reason to have these flags is when instructions don't have patterns.

I'm fairly certain that Jakob put in an error (warning?) when tablegen detects redundant flags.

Evan

Jakob_Stoklund_Olese · January 7, 2014, 6:47pm

Tablegen will emit an error if the specified flags are inconsistent with the patterns.

If you clear the guessInstructionProperties flag, it will also refuse to infer flags from patterns, while still verifying the manually specified flags against the patterns when possible.

Thanks,
/jakob

resistor · January 7, 2014, 7:09pm

I’d like to throw out that having the inference present can be a massive problem at time. It’s reasonably common to have a multi class that is reused for a number of different instructions, say different types of load. Some of the instantiations of the class have a pattern (say a normal i32 load) and some don’t (load of an illegal type, for example). In this case, you need to set the isLoad flag on the defs in the multiclass in order the make sure the latter get it properly, but you’ll get warnings (errors?) from tblgen about redundant flags on the former. The typical workaround is to duplicate the multiclass for with-flags and without-flags, which is even more useless duplication than having the flags ever was.

—Owen

James_Grosbach · January 7, 2014, 7:20pm

+1

Finkel_Hal_J · January 7, 2014, 7:25pm

From: "Owen Anderson" <resistor@mac.com>
To: "Jakob Stoklund Olesen" <stoklund@2pi.dk>
Cc: "LLVM Developers Mailing List" <llvmdev@cs.uiuc.edu>
Sent: Tuesday, January 7, 2014 1:09:05 PM
Subject: Re: [LLVMdev] Random question about the x86 backend (and backends in general I suppose)

From: "Craig Topper" < craig.topper@gmail.com >
To: "Chandler Carruth" < chandlerc@google.com >
Cc: "LLVM Developers Mailing List" < llvmdev@cs.uiuc.edu >
Sent: Monday, December 30, 2013 2:29:50 PM
Subject: Re: [LLVMdev] Random question about the x86 backend (and
backends in general I suppose)

I can't speak directly to the questions themselves, but I'll ask a
couple back. When you say that some instructions are missing
mayLoad, do these instructions have patterns? Tablegen can infer
mayLoad/mayStores/hasSideEffects from patterns so it doesn't always
need to be listed explicitly in the td files.

Having recently audited these flags in the PowerPC backend, I highly
recommend looking at these from the *GenInstrInfo.inc file directly.
I find this much easier. In theory, we'd like to move away from the
pattern-based flag inference. Once a target is free of dependence on
the inference rules, it can set bit guessInstructionProperties = 0;
to turn them off completely (see class InstrInfo in Target.td).

In MHO, we should try to avoid redundancy as much as possible. The
only reason to have these flags is when instructions don't have
patterns.

I'm fairly certain that Jakob put in an error (warning?) when
tablegen detects redundant flags.

Tablegen will emit an error if the specified flags are inconsistent
with the patterns.

If you clear the guessInstructionProperties flag, it will also refuse
to infer flags from patterns, while still verifying the manually
specified flags against the patterns when possible.

I’d like to throw out that having the inference present can be a
massive problem at time. It’s reasonably common to have a multi
class that is reused for a number of different instructions, say
different types of load. Some of the instantiations of the class
have a pattern (say a normal i32 load) and some don’t (load of an
illegal type, for example). In this case, you need to set the isLoad
flag on the defs in the multiclass in order the make sure the latter
get it properly, but you’ll get warnings (errors?) from tblgen about
redundant flags on the former. The typical workaround is to
duplicate the multiclass for with-flags and without-flags, which is
even more useless duplication than having the flags ever was.

I'd like to second this. The problem with the current inference scheme is that it is not consistent: instructions sometimes get inferred flags, depending on how their patterns are specified, and sometimes don't. Furthermore, auditing those inferred flags is not easy, and because this is a correctness issue, they do need to be checked.

My preference would be for TableGen to warn if a flag that is implied by a pattern is missing from the instruction, and nothing more than that.

-Hal

Jakob_Stoklund_Olese · January 7, 2014, 8:29pm

Sorry, that wasn’t accurate.

With guessInstructionProperties=0, Tablegen will refuse to invent flags out of thin air for instructions that have neither patterns nor explicit flags. It will still infer flags for instructions with patterns and no explicit flags.

It will always complain when flags inferred from patterns are inconsistent with explicit flags.

Thanks,
/jakob

preames · January 8, 2014, 5:44pm

Is there an easy way to inspect the resulting flags after inference has run? It seems like this would be the best format for auditing correctness. If this doesn't already exist, adding a documentation generator which lists all the instructions, their inferred/explicit flags, and where they're defined could be a useful project.

Philip

Finkel_Hal_J · January 8, 2014, 5:53pm

From: "Philip Reames" <listmail@philipreames.com>
To: "Hal Finkel" <hfinkel@anl.gov>, "Owen Anderson" <resistor@mac.com>
Cc: "LLVM Developers Mailing List" <llvmdev@cs.uiuc.edu>
Sent: Wednesday, January 8, 2014 11:44:01 AM
Subject: Re: [LLVMdev] Random question about the x86 backend (and backends in general I suppose)

>
> I'd like to second this. The problem with the current inference
> scheme is that it is not consistent: instructions sometimes get
> inferred flags, depending on how their patterns are specified, and
> sometimes don't. Furthermore, auditing those inferred flags is not
> easy, and because this is a correctness issue, they do need to be
> checked.
>
Is there an easy way to inspect the resulting flags after inference
has
run?

Sure. You can look, in the build directory, at the lib/Target/*/*GenInstrInfo.inc file. You'll see a big array of structure initializers, one for each instruction, and a flags variable that will have something like:

0|(1<<MCID::Pseudo)|(1<<MCID::Call)|(1<<MCID::MayLoad)|(1<<MCID::UsesCustomInserter)

It's just not very user-friendly

-Hal