[PATCH] D56057: [X86] Individually simplify both operands of PMULDQ/PMULUDQ using the other entry point of SimplifyDemandedBits that allows the one use check of the root node to be suppressed.

Sun Dec 23 22:18:33 PST 2018

easyaspi314 added a comment.

In D56057#1340522 <https://reviews.llvm.org/D56057#1340522>, @craig.topper wrote:

> The really concerning thing is that this patch does the exact same thing that's done by simplifyI24 in AMDGPUISelLowering.cpp. I assume Simon like myself assumed that established infrastructure like that was doing the right thing and didn't question it too much.

Ok. Well, unfortunately, that too is probably broken.

I would test it myself, but I only have integrated on a Sandy Bridge laptop.

The good thing is that we are catching it here. It could've been a lot worse. If this got into x86 world, it would go from a visual glitch to someone getting a bill for $4,294,967,295 because of an arithmetic bug in the compiler.

Sorry if I came off sounding like a jerk, by the way.

I understand that there are a lot of patches that come in, and checking the thousands of tests is time consuming, and I don't expect every single test to be read line for line.

I was just a little shocked when I saw this patch, with hundreds of lines removed from the test results, pass review without question, even after I had already expressed my concerns with it.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56057/new/

https://reviews.llvm.org/D56057