[PATCH] D77319: [DAGCombine] Remove the getNegatibleCost to avoid the out of sync with getNegatedExpression

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang marked an inline comment as done.
steven.zhang added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:5719
+      Cost = CostX;
+      assert(Cost != NegatibleCost::Expensive);
+      return DAG.getNode(Opcode, DL, VT, NegX, Y, Flags);
----------------
qiucf wrote:
> If I understand correctly, we can make sure only internal bug in this method would hit the assert? Since we shouldn't get non-null `NegX` but expensive `CostX`.
> 
> I just mean: we used to have many asserts in this function, because we need to ensure it's 'aligned' with return value of `getNegatibleCost`. But now we removed that function and use null SDValue to represent `Expensive` results, so we don't need those asserts any more.
> 
> Only asserts like here to catch internal bugs are okay.
Thank you for the review. Yes, as the getNegatedExpression could be override by target, we need to make sure that they didn't return non-null value with expensive cost, as we make assumption that, if the return value is null, it means expensive negating.


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

https://reviews.llvm.org/D77319