[PATCH] D105432: [Analysis] Add simple cost model for strict (in-order) reductions

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:1152
+  static bool isOrderedReduction(unsigned Opcode, FastMathFlags FPFlags) {
+    if ((Opcode == Instruction::FAdd || Opcode == Instruction::FMul) &&
+        !FPFlags.allowReassoc())
----------------
dmgreen wrote:
> sdesmalen wrote:
> > david-arm wrote:
> > > sdesmalen wrote:
> > > > Why does this need to check the opcode?
> > > Sadly this is a result of passing using FastMathFlags to determine the algorithm. The flags are usually empty for integer operations, which means the allow reassoc flag will not be set. If we don't check the opcode then we end up using strict reductions for all integer operations.
> > If the default FMF constructor results in setting `AllowReassoc=false`, then I think that's a more concrete argument for using `Optional<FastMathFlags>`, i.e. if there are no fast-math flags, there is nothing that can ask for a 'strict ordering', and so the function would return false.
> The only reduction that _can_ require strict orderings are fp operations without reassoc set. It's not a product of the fast math flags alone that means codegen will have to expand in-order. It is a product of the opcode _and_ fast math flags.
> 
> Integer reduction cannot be expanded in-order, and the cost needn't ever look at FMF's for them.
I think @sdesmalen's point is more that we now have a bit of an ugly check for the opcode in addition to the flags. I agree that using an Optional<FastMathFlags> here is nicer than always having to look for a FP opcode.


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

https://reviews.llvm.org/D105432