[PATCH] D52707: Switch optimization in IR for known maximum switch values

Ayonam Ray via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 2 14:32:48 PDT 2018


ayonam added inline comments.


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:4859
+         ValueWidth)
+    MaxSwitchValue = (1 << Known.countMaxTrailingOnes()) - 1;
+
----------------
efriedma wrote:
> ayonam wrote:
> > efriedma wrote:
> > > Can this shift overflow?
> > countMaxTrailingOnes will not exceed the number of bits in the switch expression type (local variable ValueWidth above).  But then the '1' should probably be '1LL' and MaxSwitchValue should be of type 'long long' to ensure that all possible value widths are taken care of.
> The LLVM IR "switch" instruction allows the operand to be any width.  (And even in C, you can write a switch using __uint128_t.)  You probably want to use APInt here.
> 
> And actually, I'm not sure why you're shifting here in the first place; the maximum possible value is exactly "~Known.Zero".
Yes, I didn't factor in the __uint128_t type.  I will change the code to use the ~Known.Zero value for the Max Switch Value.  

Using the APInt type would be safer.  I will make the necessary changes.


Repository:
  rL LLVM

https://reviews.llvm.org/D52707





More information about the llvm-commits mailing list