[PATCH] D43762: [IPSCCP] Use constant range information for comparisons of parameters.

Florian Hahn via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 20 06:57:13 PDT 2018



On 12/07/2018 03:12, Eric Christopher wrote:
> I've gone ahead and reverted this for now in r336877. Happy to help if 
> we need anything more here.
> 
> -eric
> 
> On Wed, Jul 11, 2018 at 3:39 PM Jordan Rupprecht via llvm-commits 
> <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> 
>     It looks like this commit caused a miscompile regression (with -O1
>     or greater), reduced to this example:
> 
>     namespace {
>     template <typename T1, typename T2>
>     bool Same(const T1& v1, const T2& v2) {
>        return v1 == v2;
>     }
> 
>     class Thing {
>       public:
>        enum Color { kPurple, kYellow };
>        enum Numbers { kZero };
> 
>        static bool CheckFoo(int x, Color color) {
>          if (!Same(kZero, x)) __builtin_abort();
>          if (color == kPurple) __builtin_printf("color is purple\n");
>          return true;
>        }
>     };
> 
>     class SomeThing : public Thing {
>       public:
>        static void DoBar() {
>          if (!CheckFoo(0, kPurple)) {
>            __builtin_printf("This should never happen\n");
>          }
>        }
>     };
> 
>     class OtherThing : public Thing {
>       public:
>        static void DoBar() {
>          if (!CheckFoo(0, kYellow)) {
>            __builtin_printf("This should never happen\n");
>          }
>        }
>     };
>     }  // namespace
> 
>     int main(int argc, char** argv) {
>        SomeThing::DoBar();
>        OtherThing::DoBar();
>        return 0;
>     }
> 
>     Output:
>     $ <clang built @r336098> -O1 regression.cc && ./a.out
>     color is purple
>     This should never happen
>     This should never happen
>     $ <clang built @r336097> -O1 regression.cc && ./a.out
>     color is purple
>     It looks like this might just be uncovering a bug in an earlier
>     revision (r335588), but we're not sure yet. Mind if we revert this
>     change until we figure out what's going on?
> 
>     (I also left this comment on https://reviews.llvm.org/rL336098,
>     reposting here for visibility)

I've recommitted the patch again as r337548. The underlying issue was 
just exposed by the commit and was fixed in rL337507.

Thanks again for the excellent reproducer and please let me know if 
there are any more problems with the non-reduced code.


Cheers,
Florian


More information about the llvm-commits mailing list