[llvm-dev] Nested ADJCALLSTACK UP/DOWN allowed?

[Tim Neumann via llvm-dev]

Hi all,

I once again ran into an issue / assertion while playing around with
the experimental LLVM AVR backend. This time, however I'm not sure if
the issue is with LLVM in general or with the AVR backend. I believe
that where the issue is boils down to the following question (which I
don't know / couldn't find the answer to):

*Are nested `ADJCALLSTACK` `UP`/`DOWN` instructions allowed / supposed
to be supported? i.e. is `down/down/up/up` supposed to be a valid
sequence of instructions or only `down/up/down/up`?*

---

If the answer is that they are _not_ supposed to be nested, then I
believe that the issue is with LLVM in general. Assuming that this is
the case, or for those interested, I have included a summary of the
original issue [0] below.

Consider the SUnit DAG of compiling [1] with `llc -O1` before [2] and
after [3] `PrescheduleNodesWithMultipleUses` is run on it. Notice how
the dependencies have been rearranged so that `SU2` now depends on the
stores rather than the `CopyFromReg`s, which also means that both
`ADJCALLSTACKUP`s need to be scheduled before `SU2`. This
transformation causes the issue below.

The actual symptoms:

The first assertion I hit was `Assertion failed: (BestRC && "Couldn't
find the register class"), function getMinimalPhysRegClass,
TargetRegisterInfo.cpp:203` in
`ScheduleDAGRRList::PickNodeToScheduleBottomUp`, which was triggered
because the `MVT` being passed in was "Glue" (for which there
obviously is no register class, at least if I understand everything
correctly).

We are getting "Glue" as the `MVT` because `getPhysicalRegisterVT`,
which is called to produce it, is returning bad data due to a
"missing" assertion. Note that I'm kind of guessing how the code in
`getPhysicalRegisterVT` is supposed to behave, in particular I'm
guessing that the `for`-loop is only supposed to terminate from the
`break` and not due to running out of elements to iterate over. I'm
somewhat confident in this guess since adding an assertion like in [5]
does not cause any tests in the test suite to fail. It does, however,
trigger instead of the assertion above.

In any case, what is happening to produce the "Glue" is that the
register passed in is 53, which is not a real register but one passed
the end of valid register IDs. As I understand the code, this is
sometime called a "CallResource". This means that the `for`-loop in
`getPhysicalRegisterVT` never breaks and thus `NumRes` is one passed
the end of the data about "implicit defs", which just happens to be
"Glue".

Anyway, that's how I understand the issue currently. Thanks for
reading, especially if you've made it this far.

[0]: https://github.com/avr-rust/rust/issues/111
[1]: https://gist.github.com/501819422631149c3ab2b8cc0b15c98d
[2]: https://user-images.githubusercontent.com/1178249/47268656-bb872c00-d553-11e8-9c0e-be30c4cac595.png
[3]: https://user-images.githubusercontent.com/1178249/47256523-31bb5e00-d482-11e8-84a8-25a546aba654.png
[4]: https://gist.github.com/2255d0d389b30edbaea43e26bbdbdd1c

Regards
Tim