[PATCH] D29937: [RISCV 15/n] Implement lowering of ISD::SELECT_CC

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: lib/Target/RISCV/RISCVISelLowering.cpp:209
+  // Copy1MBB:
+  //  %Result = phi [ %FalseValue, Copy0MBB ], [ %TrueValue, ThisMBB ]
+  // ...
----------------
reames wrote:
> Hm, as written, isn't this basically making a static prediction the true value is common?  Is there profiling available on the SETCC instruction?
I believe MachineBlockPlacement should handle this. RISC-V compilers should assume a forward branch is predicted not-taken, so the assumption would actually be the false path is the common case.


================
Comment at: test/CodeGen/RISCV/select-cc.ll:5
+; CHECK-LABEL: foo:
+; CHECK: beq a0, a2, .LBB{{.+}}
+  %val1 = load volatile i32, i32* %b
----------------
reames wrote:
> Please expand your test to cover the whole diamond.
The comment was incorrect, we're actually inserting a triangle (though as you point out, the confusing variable naming makes it really hard to see what is going on!)


https://reviews.llvm.org/D29937