[PATCH] D156444: [llvm][RISCV][IR] Zext flag in IR for RISC-V

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

I don't see tests for constantexpr.



================
Comment at: llvm/include/llvm/IR/Instruction.h:372
 
+  /// Set or clear the sext flag on this instruction, which must be an operator
+  /// which supports this flag. In our case NonNegOperator defined in Operator.h.
----------------
"sext flag" -> "nneg flag"


================
Comment at: llvm/include/llvm/IR/Instruction.h:382
 
+  /// Determine whether the the value of this Instr is NonNeg.
+  bool hasNonNeg() const LLVM_READONLY;
----------------
"Determine whether the nneg flag is set."


================
Comment at: llvm/lib/AsmParser/LLLexer.cpp:567
   KEYWORD(nsw);
+  KEYWORD(nneg);
   KEYWORD(exact);
----------------
I think this list should try to be the same order as LLVMToken.h where you have this after `inbounds`


================
Comment at: llvm/lib/AsmParser/LLParser.cpp:6381
+    bool NonNeg = EatIfPresent(lltok::kw_nneg);
+    bool Res = parseCast(Inst, PFS, KeywordVal, NonNeg);
+    if (Res != 0) {
----------------
We can just do `return parseCast` I think?


================
Comment at: llvm/lib/AsmParser/LLParser.cpp:7181
   Inst = CastInst::Create((Instruction::CastOps)Opc, Op, DestTy);
+  if(NonNeg) Inst->setNonNeg(true);
   return false;
----------------
Space after `if`


================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:3216
     }
-    case bitc::CST_CODE_CE_CAST: {  // CE_CAST: [opcode, opty, opval]
+    case bitc::CST_CODE_CE_CAST: { // CE_CAST: [opcode, opty, opval]
       if (Record.size() < 3)
----------------
Don't change the formatting of this line


================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:3224
+        uint8_t Flags = 0;
+        if (Record.size() >= 4 &&  (Record[3] & (1 << bitc::NNO_NON_NEG))) 
+          Flags |= NonNegOperator::NonNeg;
----------------
Extra space after `&&`


================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:4894
     }
-    case bitc::FUNC_CODE_INST_CAST: {    // CAST: [opval, opty, destty, castopc]
+    case bitc::FUNC_CODE_INST_CAST: { // CAST: [opval, opty, destty, castopc]
       unsigned OpNum = 0;
----------------
Don't change the formatting of this line


================
Comment at: llvm/lib/IR/Instruction.cpp:384
+  if (auto *NNO = dyn_cast<NonNegOperator>(V))
+    if(isa<NonNegOperator>(this))
+      setNonNeg(NNO->hasNonNeg());
----------------
Space after `if`


================
Comment at: llvm/lib/IR/Instruction.cpp:413
+  if (auto *NNO = dyn_cast<NonNegOperator>(V))
+    if(isa<NonNegOperator>(this))
+      setNonNeg(hasNonNeg() && NNO->hasNonNeg()); 
----------------
space after `if`


================
Comment at: llvm/test/Bitcode/constantsTest.3.2.ll:124
   ret void
-}
\ No newline at end of file
+}
----------------
The only change to this file seems to be adding a new line. I don't think we should touch this file in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156444