[PATCH] D73200: GlobalISel: Add computeKnownBitsForTargetInstr

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp:431
 
-  // TODO: Handle target instructions
   // TODO: Fall back to known bits
----------------
This TODO hasn't quite been handled. It's also intended to cover the case where you have target specific instructions (e.g. AArch64::ADDWrr), pseudo-instructions, or target-specific generic opcodes.

To cover this case we also need to call `TL.computeNumSignBitsForTargetInstr()` for the `default:` case


================
Comment at: llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp:433
+      TL.computeNumSignBitsForTargetInstr(*this, R, DemandedElts, MRI, Depth);
+    if (NumBits > 1)
+      FirstAnswer = std::max(FirstAnswer, NumBits);
----------------
FirstAnswer is 1 so this if-statement is already inside the std::max


================
Comment at: llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp:434
+    if (NumBits > 1)
+      FirstAnswer = std::max(FirstAnswer, NumBits);
     break;
----------------
Why update a variable that and return that rather than just returning the answer as per the other code paths?


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

https://reviews.llvm.org/D73200