[IR] Allow fast math flags on calls with floating point array type.

[foad]

Summary

This extends the rules for when a call instruction is deemed to be an
FPMathOperator, which is based on the type of the call (i.e. the return
type of the function being called). Previously we only allowed
floating-point and vector-of-floating-point types. Now we also allow
arrays (nested to any depth) of floating-point and
vector-of-floating-point types.

This was motivated by llpc, the pipeline compiler for AMD GPUs
(https://github.com/GPUOpen-Drivers/llpc). llpc has many math library
functions that operate on vectors, typically represented as <4 x float>,
and some that operate on matrices, typically represented as
[4 x <4 x float>], and it's useful to be able to decorate calls to all
of them with fast math flags.

[spatel]

The LangRef needs edits for this change.
I think this patch is also allowing the following constructs, but not testing for them.

define [3 x float] @arrayselect(i1 %cond, [3 x float] %x, [3 x float] %y) {
  %sel = select ninf i1 %cond, [3 x float] %x, [3 x float] %y
  ret [3 x float] %sel
}

define [1 x float] @arrayphi(i1 %cond, [1 x float] %x, [1 x float] %y) {
entry:
  br i1 %cond, label %t, label %f

t:
  br label %exit

f:
  br label %exit

exit:
  %phi = phi nnan [1 x float] [ %x, %t ], [ %y, %f ]
  ret [1 x float] %phi
}

[foad]

Update langref.

[foad]

In D69161#1714388, @spatel wrote:

I think this patch is also allowing the following constructs, but not testing for them.

Are there any existing tests for fast math flags on select or phi, that I could extend? I didn't even know they were allowed because they weren't mentioned in this part of the docs (https://llvm.org/docs/LangRef.html#fast-math-flags) and they aren't explicitly listed in the implementation of FPMathOperator::classof.

Incidentally, surely FPMathOperator::classof would be better implemented as a whitelist rather than a blacklist? At the moment I'm sure it is unintentionally allowing fast math flags on some instructions like ExtractValue. (This is now D69176.)

[foad]

Rebase.

[spatel]

In D69161#1714549, @foad wrote:

In D69161#1714388, @spatel wrote:

I think this patch is also allowing the following constructs, but not testing for them.

Are there any existing tests for fast math flags on select or phi, that I could extend? I didn't even know they were allowed because they weren't mentioned in this part of the docs (https://llvm.org/docs/LangRef.html#fast-math-flags) and they aren't explicitly listed in the implementation of FPMathOperator::classof.

Yes, select/phi are relatively recent additions, and we missed that LangRef update. Test diff ideas may be in these reviews:
D61917
D67564
D68748

[foad]

Rebase after D69176.

[arsenm]

Also should have some real IR tests that show this parses and preserves the flags

[foad]

Update bitcode reader and ll parser and add tests.

[foad]

Ping. I think I've added all the tests that folks asked for -- and fixed bugs they exposed in LLParser and BitcodeReader.

[spatel]

llvm/test/Bitcode/compatibility.ll

942 ↗

(On Diff #225859)

Duplicate this test with the array equivalents, so we are sure that path is round-tripping as expected?

[foad]

Add a bitcode compatibility test for fast math flags on calls returning
array types.

[spatel]

LGTM

llvm/docs/LangRef.rst

 deemed to occur on the edge from the corresponding predecessor block to
 the current block (but after any definition of an '``invoke``'
 instruction's return value on the same edge).
 
 The optional ``fast-math-flags`` marker indicates that the phi has one
 or more :ref:`fast-math-flags <fastmath>`. These are optimization hints
 to enable otherwise unsafe floating-point optimizations. Fast-math-flags
 are only valid for phis that return a floating-point scalar or vector
-type.
+type, or an array (nested to any depth) of floating-point scalar or vector
+types.
 
 Semantics:
 """"""""""
 
 At runtime, the '``phi``' instruction logically takes on the value
 specified by the pair corresponding to the predecessor basic block that
 executed just prior to the current block.
 

 
 The '``select``' instruction requires an 'i1' value or a vector of 'i1'
 values indicating the condition, and two values of the same :ref:`first
 class <t_firstclass>` type.
 
 #. The optional ``fast-math flags`` marker indicates that the select has one or more
    :ref:`fast-math flags <fastmath>`. These are optimization hints to enable
    otherwise unsafe floating-point optimizations. Fast-math flags are only valid
-   for selects that return a floating-point scalar or vector type.
+   for selects that return a floating-point scalar or vector type, or an array
+   (nested to any depth) of floating-point scalar or vector types.
 
 Semantics:
 """"""""""
 
 If the condition is an i1 and it evaluates to 1, the instruction returns
 the first value argument; otherwise, it returns the second value
 argument.
 

 
 #. The optional ``notail`` marker indicates that the optimizers should not add
    ``tail`` or ``musttail`` markers to the call. It is used to prevent tail
    call optimization from being performed on the call.
 
 #. The optional ``fast-math flags`` marker indicates that the call has one or more
    :ref:`fast-math flags <fastmath>`, which are optimization hints to enable
    otherwise unsafe floating-point optimizations. Fast-math flags are only valid
-   for calls that return a floating-point scalar or vector type.
+   for calls that return a floating-point scalar or vector type, or an array
+   (nested to any depth) of floating-point scalar or vector types.
 
 #. The optional "cconv" marker indicates which :ref:`calling
    convention <callingconv>` the call should use. If none is
    specified, the call defaults to using C calling conventions. The
    calling convention of the call must match the calling convention of
    the target function, or else the behavior is undefined.
 #. The optional :ref:`Parameter Attributes <paramattrs>` list for return
    values. Only '``zeroext``', '``signext``', and '``inreg``' attributes

llvm/include/llvm/IR/Operator.h

static bool classof(const Value *V) {
     //        should not be treated as a math op, but the other opcodes should.
     //        This would make things consistent with Select/PHI (FP value type
     //        determines whether they are math ops and, therefore, capable of
     //        having fast-math-flags).
     case Instruction::FCmp:
       return true;
     case Instruction::PHI:
     case Instruction::Select:
-    case Instruction::Call:
-      return V->getType()->isFPOrFPVectorTy();
+    case Instruction::Call: {
+      Type *Ty = V->getType();
+      while (ArrayType *ArrTy = dyn_cast<ArrayType>(Ty))
+        Ty = ArrTy->getElementType();
+      return Ty->isFPOrFPVectorTy();
+    }
     default:
       return false;
     }
   }
 };
 
 /// A helper template for defining operators for individual opcodes.
 template<typename SuperClass, unsigned Opc>

llvm/unittests/IR/InstructionsTest.cpp

TEST(InstructionsTest, PhiMightNotBeFPMathOperator) {
   Instruction *I = Builder.CreatePHI(Builder.getInt32Ty(), 0);
   EXPECT_FALSE(isa<FPMathOperator>(I));
   I->deleteValue();
   Instruction *FP = Builder.CreatePHI(Builder.getDoubleTy(), 0);
   EXPECT_TRUE(isa<FPMathOperator>(FP));
   FP->deleteValue();
 }
 
+TEST(InstructionsTest, FPCallIsFPMathOperator) {
+  LLVMContext C;
+
+  Type *ITy = Type::getInt32Ty(C);
+  FunctionType *IFnTy = FunctionType::get(ITy, {});
+  Value *ICallee = Constant::getNullValue(IFnTy->getPointerTo());
+  std::unique_ptr<CallInst> ICall(CallInst::Create(IFnTy, ICallee, {}, ""));
+  EXPECT_FALSE(isa<FPMathOperator>(ICall));
+
+  Type *VITy = VectorType::get(ITy, 2);
+  FunctionType *VIFnTy = FunctionType::get(VITy, {});
+  Value *VICallee = Constant::getNullValue(VIFnTy->getPointerTo());
+  std::unique_ptr<CallInst> VICall(CallInst::Create(VIFnTy, VICallee, {}, ""));
+  EXPECT_FALSE(isa<FPMathOperator>(VICall));
+
+  Type *AITy = ArrayType::get(ITy, 2);
+  FunctionType *AIFnTy = FunctionType::get(AITy, {});
+  Value *AICallee = Constant::getNullValue(AIFnTy->getPointerTo());
+  std::unique_ptr<CallInst> AICall(CallInst::Create(AIFnTy, AICallee, {}, ""));
+  EXPECT_FALSE(isa<FPMathOperator>(AICall));
+
+  Type *FTy = Type::getFloatTy(C);
+  FunctionType *FFnTy = FunctionType::get(FTy, {});
+  Value *FCallee = Constant::getNullValue(FFnTy->getPointerTo());
+  std::unique_ptr<CallInst> FCall(CallInst::Create(FFnTy, FCallee, {}, ""));
+  EXPECT_TRUE(isa<FPMathOperator>(FCall));
+
+  Type *VFTy = VectorType::get(FTy, 2);
+  FunctionType *VFFnTy = FunctionType::get(VFTy, {});
+  Value *VFCallee = Constant::getNullValue(VFFnTy->getPointerTo());
+  std::unique_ptr<CallInst> VFCall(CallInst::Create(VFFnTy, VFCallee, {}, ""));
+  EXPECT_TRUE(isa<FPMathOperator>(VFCall));
+
+  Type *AFTy = ArrayType::get(FTy, 2);
+  FunctionType *AFFnTy = FunctionType::get(AFTy, {});
+  Value *AFCallee = Constant::getNullValue(AFFnTy->getPointerTo());
+  std::unique_ptr<CallInst> AFCall(CallInst::Create(AFFnTy, AFCallee, {}, ""));
+  EXPECT_TRUE(isa<FPMathOperator>(AFCall));
+
+  Type *AVFTy = ArrayType::get(VFTy, 2);
+  FunctionType *AVFFnTy = FunctionType::get(AVFTy, {});
+  Value *AVFCallee = Constant::getNullValue(AVFFnTy->getPointerTo());
+  std::unique_ptr<CallInst> AVFCall(
+      CallInst::Create(AVFFnTy, AVFCallee, {}, ""));
+  EXPECT_TRUE(isa<FPMathOperator>(AVFCall));
+
+  Type *AAVFTy = ArrayType::get(AVFTy, 2);
+  FunctionType *AAVFFnTy = FunctionType::get(AAVFTy, {});
+  Value *AAVFCallee = Constant::getNullValue(AAVFFnTy->getPointerTo());
+  std::unique_ptr<CallInst> AAVFCall(
+      CallInst::Create(AAVFFnTy, AAVFCallee, {}, ""));
+  EXPECT_TRUE(isa<FPMathOperator>(AAVFCall));
+}
+
 TEST(InstructionsTest, FNegInstruction) {
   LLVMContext Context;
   Type *FltTy = Type::getFloatTy(Context);
   Constant *One = ConstantFP::get(FltTy, 1.0);
   BinaryOperator *FAdd = BinaryOperator::CreateFAdd(One, One);
   FAdd->setHasNoNaNs(true);
   UnaryOperator *FNeg = UnaryOperator::CreateFNegFMF(One, FAdd);
   EXPECT_TRUE(FNeg->hasNoNaNs());