[PATCH] D88193: [SLP] Remove LHS and RHS from OperationData.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6484
     bool operator==(const OperationData &OD) const {
-      assert(((Kind != OD.Kind) || ((!LHS == !OD.LHS) && (!RHS == !OD.RHS))) &&
+      assert(((Kind != OD.Kind) || (Opcode != 0 && OD.Opcode != 0)) &&
              "One of the comparing operations is incorrect.");
----------------
I'm not entirely sure what this assert was trying to check before.

As far as I can tell there are 3 states OperationData could have
- Kind is RK_None and Opcode is 0 and LHS/RHS wre. 
- Kind is RK_None and Opcode is non-zero and LHS/RHS are null
- Kind is not RK_None and Opcode is non-zero and LHS/RHS are non-null

So I think this was previously saying that if the Kinds are equal then the LHS/RHS in both should be null or non-null. Maybe I should have changed it to (Kind != OD.Kind) || ((Opcode != 0) == (OD.Opcode != 0))?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6499
              "One of the comparing operations is incorrect.");
-      return this == &OD || (Kind == OD.Kind && Opcode == OD.Opcode);
     }
----------------
I remove this pointer comparison. I didn't see any reason checking the fields wasn't ok this and OD being the same object.


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

https://reviews.llvm.org/D88193