[PATCH] Switch to Select range optimization

Hans Wennborg hans at chromium.org
Fri Nov 14 14:01:51 PST 2014


On Fri, Nov 14, 2014 at 12:17 PM, Marcello Maggioni <hayarms at gmail.com> wrote:
> Ok, thanks Hans!
> I will do it next time. I usually use "git diff" for generating the diff , I'll take a look at Arcanist too though.

I've never used Arcanist, I just use git diff -U99999.

>
> ================
> Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3540
> @@ +3539,3 @@
> +  // the minimum and maximum cases and the zeroes and ones for this case group.
> +  for (auto &Case : Cases) {
> +    bool CaseMatchesCondPattern =
> ----------------
> hans wrote:
>> I don't see any need for auto here..
>>
>> for (ConstantInt *Case : Cases)
>>
>> would be clearer, and it avoids making Case a reference to a pointer type, which seems unnecessary.
> Will do.
>
> ================
> Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3721
> @@ +3720,3 @@
> +  bool DefaultCanTrigger = (((1 << (BitWidth - NumKnownCondBits)) -
> +                        (FirstGroupValidCases + SecondGroupValidCases)) != 0);
> +  DefaultCanTrigger &= (bool)DefaultResult;
> ----------------
> hans wrote:
>> Using known bits to figure out if the default case can trigger is clever. Other optimizations would benefit from this too, so I think SimplifyCFG should have a separate transformation that does this analysis and makes the default case unreachable. That way more optimizations benefit, and the code here becomes simpler.
> Do you think I should hoist it to a separate static function in SimplifyCFG?

It should be a separate simplification that
SimplifyCFGOpt::SimplifySwitch tries to do before we do the fancy
switch-to-select or switch-to-lookup table transforms.

There is already EliminateDeadSwitchCases that computes known bits;
maybe it should be part of that (though we shouldn't eliminate the
default case, just mark it unreachable).

 - Hans




More information about the llvm-commits mailing list