Nested instruction patterns rejected by GlobalISel when having registers in Defs

Hi,

I am in the process of porting our target to GlobalISel, and have encountered a problem. Nearly all instructions in our instruction set make modifications to a CC register, and hence are defined as follows:

let …, Defs = [CCReg] in

def shfts_a32_imm7: Instruction<(outs OurRC:$dst), …>;

What’s more, many of these instructions have patterns where the instruction itself appears inside a nested tree, e.g.:

def Pat<(source pattern …),

(sext_a32 (INSERT_SUBREG (…), (shfts_a32_imm7 OurRC:$src, Imm7:$imm), …>;

Now to the problem: When TableGen processes the instruction above, it includes the CCReg in the Defs field along with the registers appearing in outs, thereby indicating that shfts_a32_imm7 produces two results. Currently, the GlobalISel-backend in TableGen requires that nested instructions appearing in the output pattern produce exactly one result. Consequently, TableGen rejects many of our patterns. But in reality, the instruction really only produces a single result and therefore this pattern should be allowed.

So I wonder, how should registers appear in Defs be treated? Are they equal to those appearing in outs, and therefore interchangeable, or is it valid to disambiguate between them and therefore modify TableGen to only consider outs as the result of interest when processing the patterns?

Gabriel Hjort Åkerlund

Hi Gabriel,

I’m working on a downstream target which uses GlobalISel and we have many patterns with instructions that also define a system register as a side-effect and use them without any problem. Since CCReg is not an actual output of the instruction, but an implicit definition, GlobalISel should have no trouble with it, so I’m guessing your problem lies somewhere. Have you tried running the tablegen command manually and looked at the output there?

The command is llvm-tblgen -gen-global-isel <couple of -I flags> <your_target>.td --write-if-changed --warn-on-skipped-patterns

I can’t tell you exactly what -I flags you’ll need but if you run ninja in verbose mode or look at the ninja build log, you should be able to see what is being used.

Word of caution however: sometimes TableGen gives you a very clear error message indicating what is wrong, sometimes it gives you a very cryptic error message. And sometimes it doesn’t even give you that and behave as if everything is a-ok while it still hasn’t included your pattern. I have lost countless hours trying to debug TableGen patterns with GlobalISel and there’s still a lot of stuff that GlobalISel unfortunately does not support yet in TableGen. So be prepared to write some C++ code for the unsupported cases for the moment.

Cheers,

Dominik

Hi Dominik,

Thanks for your reply.

In my case, the Defs is the cause of the problem. Or rather, it is part of the problem, because when I remove it from the instruction TableGen gives me a different error message which concerns a part which is deeper into the pattern tree, so at least it is able to proceed beyond that part of the pattern. I have also stepped TableGen inside gdb and verified that having Defs causes GlobalISel to include CCReg in the Types field of the TreePatternNode corresponding to the instruction, which is what GlobalISel looks at to subsequently reject the pattern on basis that the instruction produces multiple results.

But from your comment, I take it that the Defs field should never be considered actual output, is that correct? If so, I find it strange that CodeGenDAGPatterns, which parses the patterns, takes the CCReg into consideration as additional results. I am tempted to modify that part of the code, but maybe I’m missing some invariant that’s not immediately evident…

Cheers,

Gabriel

Hi Gabriel,

Your comment made me take a look at our instruction definitions and patterns in a little bit more detail. And while we do use nested patterns with INSERT_SUBREG, apparently none of those patterns use instructions with implicit-defs. Sorry for misleading you there by a wrong assumption on my part.

However, I still find it strange that TableGen should reject such a pattern. I’m really not sure if this is simply an overlooked use-case or if there is some real reasoning behind this logic. If it were an actual output register I could understand it, but since this is an implicit one it should not impact the pattern in my opinion.

Sorry for not being able to help out with the actual problem after all!

Cheers,

Dominik

  • Daniel who knows the most about the table gen importer

The implicit defs don’t participate much in the patterns. For the most part it’s just read into CodeGenInstruction::ImplicitDefs and then GlobalISel gathers and adds them all at the end. I think I do see the code Gabriel is referring to though.
GetNumNodeResults() has:
if (InstInfo.HasOneImplicitDefWithKnownVT(CDP.getTargetInfo()) !=MVT::Other)
++NumDefsToAdd;
and ultimately this VT gets into Types via UpdateNodeType(). I have no idea why this code does this as I don’t really see why the VT matters to how you treat an implicit def. This code dates back to 2010 (r99726) and that commit called it ‘funky logic’ so it’s unlikely that we’ll find someone that remembers it.

Does your CCReg need to have a specific type? If not, then you could make it MVT::Other and the problem should go away.

Have you looked into how DAGISel supports these patterns? This code is common between DAGISel and GlobalISel so given that DAGISel works it must be doing something to handle this that’s missing from GlobalISel.