Audit Legalizer about llvm-mos

[mysterymath]

Well, all the code that gets sent through comes from exactly two files: CallLowering.cpp and IRTranslator.cpp.
It'd be a matter of walking through each of the buildXXX() calls in each to see what it's capable of spitting out. Searching for TargetOpcode:: really.

The main point of doing something like this would be "bulk issue reduction"; we'll invariably miss some things here and (hopefully) catch them in the LLVM test suite, but debugging those breakages will probably be a significantly harder process than grepping the 3k lines or so of translator code here.

EDIT: AFAIK, nothing's blocking this work.

[johnwbyrd]

Other than by running a bunch of code through it, how do you suggest this audit should take place?

[johnwbyrd]

Running the enclosed program through clang, gives this failure in the legalizer:

LLVM ERROR: unable to legalize instruction: %33:(s1) = G_ICMP intpred(ult), %8:(s16), %32:_ (in function: set_entities)
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0. Program arguments: c:/git/llvm-mos/build/bin/llc --march=mos game.ll

1.  Running pass 'Function Pass Manager' on module 'game.ll'.
   

2.  Running pass 'Legalizer' on function '@set_entities'
   

It's not clear why the legalizer is being asked to do this, since you only signed up to receive legalization requests for s1 and s8.

game.zip

[johnwbyrd]

There may be two problems here.

In addition to the legalization problem, it's not clear why clang should be emitting so many references to i32's in the getelementptr instructions.

[mysterymath]

It's not clear why the legalizer is being asked to do this, since you only signed up to receive legalization requests for s1 and s8.

Ah, I think there's a bit of a misunderstanding here about what MOSLegalizerInfo.cpp does. When we say in MOSLegalizerInfo.cpp:

  getActionDefinitionsBuilder(G_ICMP)
      .legalFor({{S1, S8}})
      .minScalar(1, S8)
      .narrowScalarFor({{S1, S16}}, changeTo(1, S8))
      .narrowScalarFor({{S1, S32}}, changeTo(1, S16))
      .narrowScalarFor({{S1, S64}}, changeTo(1, S32));

This forms a line-by-line script for what the legalizer should actually do. This reads as:

• If the output is a S1 and the inputs are all S8, it's legal, and we're done.
• Otherwise, if the inputs are smaller than S8, call the widen routine in LegalizerHelper.cpp to make them S8. Restart legalization.
• Otherwise, if the output is a S1 and the inputs are all S16, call the narrow routine in LegalizerHelper.cpp to make them S8. Restart legalization.
• Otherwise, if the output is a S1 and the inputs are all S32, call the narrow routine in LegalizerHelper.cpp to make them S16. Restart legalization.
• Otherwise, if the output is a S1 and the inputs are all S64, call the narrow routine in LegalizerHelper.cpp to make them S32. Restart legalization.
• Otherwise, crash with the error message you observed: Cannot legalize G_ICMP.

Accordingly, any cases that come into the legalizer that aren't mentioned in one of these scripts will crash the compile. Most of the above lines I just added yesterday. I'm working on getting printf.c in the SDK to pass through each phase of the code generation, and right now it's stuck on legalization.

The docs (https://llvm.org/docs/GlobalISel/Legalizer.html) are fairly good for an overview of the process, but they're notably missing any discussion of what the legalization steps like lower or narrow are actually able to handle. For that, the only solution is reading the implementation in LegalizerHelper.cpp. Luckily, that code has quite straightforward control flow paths, so it's not too difficult to gauge what's supported.

In addition to the legalization problem, it's not clear why clang should be emitting so many references to i32's in the getelementptr instructions.

The i32's in getelementptr aren't a problem IIRC; I think they're only used for constant offsets into things like structs. Clang just picked i32 for those since it was convenient, but whenever a real runtime offset is being used, it uses the offset type from the DataLayout, which is i16 for MOS. That's why the GEPs seem to alternate between i16 and i32.

EDIT: See https://llvm.org/docs/GetElementPtr.html#why-do-struct-member-indices-always-use-i32

[johnwbyrd]

Accordingly, any cases that come into the legalizer that aren't mentioned in one of these scripts will crash the compile. Most of the above lines I just added yesterday. I'm working on getting printf.c in the SDK to pass through each phase of the code generation, and right now it's stuck on legalization.

I think I knew most of that, thank you -- I actually tried the change you submitted, but there was no joy. Will investigate further.