[PATCH] D113935: [RISCV] Add test cases to prepare for overring TargetLowering::hasAndNot. NFC

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/test/CodeGen/RISCV/selectcc-to-shiftand.ll:30
+
+; Compare if negative and select of constants where one constant is zero and the other is a single bit.
+
----------------
Wrap all these and put them right up against the test?


================
Comment at: llvm/test/CodeGen/RISCV/selectcc-to-shiftand.ll:77
+
+; Flipping the comparison condition can be handled by getting the bitwise not of the sign mask.
+
----------------
Seems we fail to do this for everything except RV32 pos_sel_special_constant, though the RV64 version of that is probably just as performant.


================
Comment at: llvm/test/CodeGen/RISCV/unfold-masked-merge-scalar-variablemask.ll:952
+}
+; ============================================================================ ;
+; Negative tests. Should not be folded.
----------------



================
Comment at: llvm/test/CodeGen/RISCV/unfold-masked-merge-scalar-variablemask.ll:168
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
+; Should be the same as the previous one.
+;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
----------------
lebedev.ri wrote:
> craig.topper wrote:
> > jrtc27 wrote:
> > > I don't understand what this is referring to
> > I think it was trying to say that we should transform each of the in* functions the equivalent of the corresponding out* test.
> Yep. The `out` is the unmerged form of masked-merge, the `in` is merged form.
Right, I see now


================
Comment at: llvm/test/CodeGen/RISCV/unfold-masked-merge-scalar-variablemask.ll:232
+}
+; ============================================================================ ;
+; Commutativity tests.
----------------
jrtc27 wrote:
> From this point on there aren't blank lines around functions (and headers like this), it's all crammed together
I think you still want a blank line after each of these block headers (including the TODO), since they apply to multiple tests and not just the one they're (currently) touching


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113935