[PATCH] D39190: [ARM] tSETEND needs IsThumb

[Renato Golin via Phabricator via llvm-commits]

rengolin accepted this revision.
rengolin added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D39190#904274, @olista01 wrote:

> It's _not_ OK to select the Thumb version of this instruction in ARM mode, and that's what this patch is preventing.


Ok, this makes more sense. :)

> Normally, the T1I base class sets the Predicates list to [IsThumb], but the Requires class overrides the whole list rather than appending to it, so we need to explicitly include IsThumb in the list here.

That's ugly! But I agree it's an acceptable fix.

> The ARM version of this instruction does have IsARM in the Predicates list, so doesn't get selected for Thumb targets.

So, -mthumb selects this one just because it's the only match, and the ARM selects the arm one, by accident, just because it's the first to match. Gotcha.

> We just happen to not hit this bug at the moment because the ARM version of the instruction appears before this one in the matcher table, but that isn't actually guaranteed by anything, and other patches that I'm working on cause the order to be reversed.

Other than changing the order of the td file in a test, I can't see how this could ever be tested.

And since it's an obvious fix, LGTM. Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D39190