[PATCH] D53877: [IR] Strawman for dedicated FNeg IR instruction

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added inline comments.


================
Comment at: include/llvm/Bitcode/LLVMBitCodes.h:346
+  CST_CODE_CE_UNOP = 25,         // CE_UNOP:      [opcode, opval]
 };
 
----------------
cameron.mcinally wrote:
> It wasn't clear to me if it was safe to reorder these values. Any thoughts on this?
No.  Reordering will break bitcode compatibility.


================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:2346
+      break;
+    }
     case bitc::CST_CODE_CE_BINOP: {  // CE_BINOP: [opcode, opval, opval]
----------------
cameron.mcinally wrote:
> I'm not that familiar with the Bitcode format. Any suggestions on the best way to produce a test case for this? Or is it just the obvious, produce bitcode and make sure it works as expected?
New IR we usually test in `test/Assembler`.  You should test double-round-tripping through bitcode (llvm-as | llvm-dis | llvm-as | llvm-dis) and pass through verify-uselistorder.  Have a look at existing tests there.

I don't think it applies here, but in cases where you need to upgrade existing bitcode to something new, we check in tiny bitcode files generated with a known released version of llvm and test that it upgrades correctly in `test/Bitcode`.


Repository:
  rL LLVM

https://reviews.llvm.org/D53877