[PATCH] D81215: [VE] Support branch instructions in MC layer

[Kazushi Marukawa via Phabricator via llvm-commits]

kaz7 marked 3 inline comments as done.
kaz7 added a comment.

Thank you for your time.  Reply to comments.



================
Comment at: llvm/lib/Target/VE/VE.h:51-66
+  CC_AF = 0 + 6,     // Never
+  CC_G = 1 + 6,      // Greater
+  CC_L = 2 + 6,      // Less
+  CC_NE = 3 + 6,     // Not Equal
+  CC_EQ = 4 + 6,     // Equal
+  CC_GE = 5 + 6,     // Greater or Equal
+  CC_LE = 6 + 6,     // Less or Equal
----------------
simoll wrote:
> The only real change here is the addition of `UNKNOWN`? I guess we should undo the unrelated indentation change for the other enum entries here.
Yes.  I guess indentation was modified by clang-format, but I can re-check it.


================
Comment at: llvm/lib/Target/VE/VE.h:258
   llvm_unreachable("Invalid cond code");
+  return VECC::CC_AF;
 }
----------------
simoll wrote:
> Return after unreachable?
Yes.  This llvm_unreachable doesn't halt release mode llvm.  I had a crash problem here once, so I added this return for the case.


================
Comment at: llvm/lib/Target/VE/VEInstrInfo.cpp:71
   llvm_unreachable("Invalid cond code");
+  return VECC::UNKNOWN;
 }
----------------
simoll wrote:
> return after unreachable
Yes.  Similar to above comment.  Maybe it is safe to remove return this time, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81215