[PATCH] RegisterCoalescer: Gracefully continue if subrange merging fails.

[Matthias Braun]

+llvm-commits this time

Unfortunately this is only exposed if you have a subregister index which produces lanemasks with more than 31bits. None of the targets in trunk gets close to that.

- Matthias

> On Mar 3, 2015, at 3:52 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
> Hi Matthias,
> 
> The change looks good, just a few nitpicks.
> Any chance we could get a testcase with that, or R600 does not expose it?
> 
> Thanks,
> -Quentin
> 
> 
> REPOSITORY
>  rL LLVM
> 
> ================
> Comment at: lib/CodeGen/RegisterCoalescer.cpp:163
> @@ -162,3 +162,3 @@
>     /// lanemasks already adjusted to the coalesced register.
> -    void mergeSubRangeInto(LiveInterval &LI, const LiveRange &ToMerge,
> +    bool mergeSubRangeInto(LiveInterval &LI, const LiveRange &ToMerge,
>                            unsigned LaneMask, CoalescerPair &CP);
> ----------------
> Add a comment on what this function returns.
> 
> ================
> Comment at: lib/CodeGen/RegisterCoalescer.cpp:168
> @@ -167,3 +167,3 @@
>     /// @p LRange, @p RRange may be invalid afterwards.
> -    void joinSubRegRanges(LiveRange &LRange, LiveRange &RRange,
> +    bool joinSubRegRanges(LiveRange &LRange, LiveRange &RRange,
>                           unsigned LaneMask, const CoalescerPair &CP);
> ----------------
> Ditto.
> 
> ================
> Comment at: lib/CodeGen/RegisterCoalescer.cpp:2398
> @@ +2397,3 @@
> +  if (!LHSVals.mapValues(RHSVals) || !RHSVals.mapValues(LHSVals)) {
> +    DEBUG(dbgs() << "*** Can't join subrange! This should not happen!\n");
> +    return false;
> ----------------
> Please update the comment before that condition to explain that we currently do not handle the overflow bit.
> Update the debug output as well to print something along the same line instead of "This should not happen!”.
> 
> ================
> Comment at: lib/CodeGen/RegisterCoalescer.cpp:2539
> @@ +2538,3 @@
> +      // This shouldn't have happened :-(
> +      // However we are aware of at least one existing problem where we
> +      // can't merge subranges when multiple ranges end up in the
> ----------------
> Says that this is a limitation of the current implement instead of this shouldn’t happen, since it does happen :).
> 
> ================
> Comment at: lib/CodeGen/RegisterCoalescer.cpp:2544
> @@ +2543,3 @@
> +      // state.
> +      assert(CP.getNewRC()->getLaneMask() & 0x80000000u);
> +      DEBUG(dbgs() << "\tSubrange join aborted!\n");
> ----------------
> Add a message in the assert, e.g., this situation should happen only for the overflow bit.
> 
> http://reviews.llvm.org/D8047
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> 
>