[PATCH] D68930: [InstCombine] Shift amount reassociation in shifty sign bit test (PR43595)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D68930#1713151 <https://reviews.llvm.org/D68930#1713151>, @lebedev.ri wrote:

> Any thoughts on this?


It raises the usual questions: cost/complexity vs. benefit, and how much further do we want to take this pattern matching before moving it somewhere else (AggressiveInstCombine)?
I don't think we'll reach consensus on that in this patch, so I've just noted a small improvement.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp:29-30
+//
+// AnalysisIsSignbitExtract is special - we will only analyze whether this
+// pattern has two right-shifts that shift my one less than original bit width,
+// thus extracting original sign bit, and return the base value X if so.
----------------
Code comment typo and difficult to parse. Maybe better:
AnalyzeForSignBitExtraction indicates that we will only analyze whether this pattern has any 2 right-shifts that sum to 1 less than original bit width.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68930