[PATCH] D156444: [llvm][IR]

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

Please upload patches with full context using -U99999 per the directions here https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface



================
Comment at: llvm/lib/AsmParser/LLParser.cpp:3747
     Lex.Lex();
+    if (Opc == Instruction::ZExt) {
+      if (EatIfPresent(lltok::kw_was_sext))
----------------
```
if (Opc == Instruction::ZExt && EatIfPresent(lltok::kw_was_sext))
  WasSext = true;
```


================
Comment at: llvm/lib/AsmParser/LLParser.cpp:6355
       return true;
+
     if (Exact) cast<BinaryOperator>(Inst)->setIsExact(true);
----------------
Unrelated change?


================
Comment at: llvm/lib/AsmParser/LLParser.cpp:6382
+  case lltok::kw_zext: {
+    bool WasSext = EatIfPresent(lltok::kw_was_sext);
+    bool Res = parseCast(Inst, PFS, KeywordVal);
----------------
Can this be moved into parseCast?


================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:1472
       Constant *C;
-      if (Instruction::isCast(BC->Opcode)) {
+      if (Instruction::isCast(BC->Opcode)) { // TODO
         C = UpgradeBitCastExpr(BC->Opcode, ConstOps[0], BC->getType());
----------------
This TODO needs to removed, or needs to say what needs to be done


================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:4874
       if (OpNum < Record.size()) {
-        if (Opc == Instruction::Add ||
-            Opc == Instruction::Sub ||
-            Opc == Instruction::Mul ||
-            Opc == Instruction::Shl) {
-          if (Record[OpNum] & (1 << bitc::OBO_NO_SIGNED_WRAP))
-            cast<BinaryOperator>(I)->setHasNoSignedWrap(true);
+        if (Opc == Instruction::Add || Opc == Instruction::Sub ||
+            Opc == Instruction::Mul || Opc == Instruction::Shl) {
----------------
Is this reformatting existing code that you aren't changing?


================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:4893
+    case bitc::FUNC_CODE_INST_CAST: { // CAST: [opval, opty, destty, castopc]
+                                      // #TODO
       unsigned OpNum = 0;
----------------
TODO has no description


================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:4921
       }
+      if (OpNum < Record.size()) {
+        if (isa<WSXTOperator>(I)) {
----------------
Join conditions with `&&` to reduce nesting


================
Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:105
 /// These are manifest constants used by the bitcode writer. They do not need to
-/// be kept in sync with the reader, but need to be consistent within this file.
+/// // TODO add zext be kept in sync with the reader, but need to be consistent
+/// within this file.
----------------
This TODO is in the middle of a sentence


================
Comment at: llvm/lib/IR/Instructions.cpp:3761
   switch (op) {
-  default: return false; // This is an input error
+  default: {
+    return false; // This is an input error
----------------
This is adding curly braces to code you didn't otherwise change. Please remove


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156444