[PATCH] D82892: [NFC] Added comparision for all types in haveSameSpecialState() of Instruction.cpp

[JF Bastien via Phabricator via llvm-commits]

jfb added a comment.

I didn't check that all of these compare all the required state for all opcodes.

That being said, this still seems somewhat brittle, if less so than before. Are there sufficient tests to find future divergences?



================
Comment at: llvm/include/llvm/IR/Instruction.h:655
+  /// which have the same Opcode and same number of operands.
+  int compareSpecialState(const Instruction *I1, unsigned flags = 0) const;
+
----------------
Can you document `flags`?


================
Comment at: llvm/include/llvm/IR/Instructions.h:70
+  int compareInstSpecificProperties(const AllocaInst *AI,
+                                    bool IgnoreAlignment = false) const;
+
----------------
I prefer having an `enum class` instead of `bool` parameters. i.e.
```
enum class IgnoreAlignment { No, Yes };
int compareInstSpecificProperties(const AllocaInst *AI,
                                    IgnoreAlignment ignoreAlignment = IgnoreAlignment::No) const;
```


================
Comment at: llvm/include/llvm/IR/Instructions.h:183
+                                    bool IgnoreAlignment = false,
+                                    bool IgnoreMetaData = false) const;
+
----------------
Same here on flags.


================
Comment at: llvm/include/llvm/IR/Instructions.h:311
+  int compareInstSpecificProperties(const StoreInst *SI,
+                                    bool IgnoreAlignment = false) const;
+
----------------
Ditto.


================
Comment at: llvm/include/llvm/IR/Instructions.h:1437
+  int compareInstSpecificProperties(const CallInst *I,
+                                    bool IgnoreMetaData) const;
+
----------------
Ditto.


================
Comment at: llvm/include/llvm/IR/Instructions.h:3691
+  int compareInstSpecificProperties(const InvokeInst *I,
+                                    bool IgnoreMetaData = false) const;
+
----------------
Ditto.


================
Comment at: llvm/include/llvm/IR/Instructions.h:3903
+  int compareInstSpecificProperties(const CallBrInst *I,
+                                    bool IgnoreMetaData = false) const;
+
----------------
Ditto.


================
Comment at: llvm/lib/IR/Instructions.cpp:82
+  return 0;
+}
+
----------------
Is it expected that APInts with different bit width but the same value won't be equal?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82892