[PATCH] D38054: [PowerPC] Fix the spill issue exposed by r310809

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D38054#878633, @nemanjai wrote:

> In https://reviews.llvm.org/D38054#878445, @echristo wrote:
>
> > In https://reviews.llvm.org/D38054#877711, @hfinkel wrote:
> >
> > > I think this generally looks fine.
> > >
> > > Just as a general note on code organizations and commenting, we seem to be accumulating a pretty deep call tree here of functions that only work correctly in 64-bit mode. Not in this patch, but we should do something about this (either segregating these things into a distinct part of the file and/or adding asserts and/or comments to the 64-bit-mode-only functions.
> >
> >
> > Agreed. Also a lot of the fine points of when to extend and when not could probably be split out as well. This is definitely getting pretty unwieldy.
> >
> > -eric
>
>
> OK. I'll commit this patch as-is with the inline comment addressed.
>
> Then perhaps in a subsequent patch, I'll create something like `class GPRComparisonSelector` that uses essentially the same approach as the `BitPermutationSelector`. The `Select()` function will do the necessary checks and exit early if we're not a PPC64 target. All the functions that do the actual selection will be private. This way we should be able to avoid carelessly calling these functions under invalid conditions.
>  Does that sound like a good plan?


Yes, I think that would be better.


Repository:
  rL LLVM

https://reviews.llvm.org/D38054