[PATCH] D73200: GlobalISel: Add computeKnownBitsForTargetInstr

[Matt Arsenault via Phabricator via llvm-commits]

arsenm marked 2 inline comments as done.
arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp:431
 
-  // TODO: Handle target instructions
   // TODO: Fall back to known bits
----------------
dsanders wrote:
> 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
This is called in the default case


================
Comment at: llvm/lib/CodeGen/GlobalISel/GISelKnownBits.cpp:434
+    if (NumBits > 1)
+      FirstAnswer = std::max(FirstAnswer, NumBits);
     break;
----------------
dsanders wrote:
> Why update a variable that and return that rather than just returning the answer as per the other code paths?
This is following the structure of the DAG implementation for eventually plugging in the default handling with computeKnownBits


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

https://reviews.llvm.org/D73200