[PATCH] D40049: [PATCH] Global reassociation for improved CSE

[escha via Phabricator via llvm-commits]

escha added inline comments.


================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:2273
+          Value *Op1 = Ops[j];
+          if (std::less<Value *>()(Op1, Op0))
+            std::swap(Op0, Op1);
----------------
qcolombet wrote:
> qcolombet wrote:
> > escha wrote:
> > > arsenm wrote:
> > > > I'm not sure what std::less on a Value * means. Is this sorting by pointer value?
> > > yes. it's canonically ordering them so we don't have to worry about checking both [a,b] and [b,a].
> > We would rather use getComplexity(Value *V) for that.
> (Though you'll probably still want something to break ties)
(talked about this one offline: conclusion was std:less is okay, I think, so long as there's no ordering dependency, which there shouldn't be)


Repository:
  rL LLVM

https://reviews.llvm.org/D40049