[PATCH] D152993: [VP][RISCV] Add vp.is.fpclass and RISC-V support

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:7930
+    SDNodeFlags SDFlags;
+    SDFlags.setNoFPExcept(
+        !MF.getFunction().getAttributes().hasFnAttr(llvm::Attribute::StrictFP));
----------------
liaolucy wrote:
> arsenm wrote:
> > craig.topper wrote:
> > > arsenm wrote:
> > > > This should be unnecessary, by definition this is not a floating point operation and cannot raise FP exceptions 
> > > Doesn't this code also exist for Intrinsic::is_fpclass?
> > If it does it shouldn’t . Can either drop it or make it an unconditional true 
> I tried to delete the flags in Intrinsic::is_fpclass and got the following diff for x86. Not sure if the diff is correct. 
> 
> 
> ```
> diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> index f38957637b74..bc8b1dddef71 100644
> --- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> +++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> @@ -6575,9 +6575,6 @@ void SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I,
>      MachineFunction &MF = DAG.getMachineFunction();
>      const Function &F = MF.getFunction();
>      SDValue Op = getValue(I.getArgOperand(0));
> -    SDNodeFlags Flags;
> -    Flags.setNoFPExcept(
> -        !F.getAttributes().hasFnAttr(llvm::Attribute::StrictFP));
>      // If ISD::IS_FPCLASS should be expanded, do it right now, because the
>      // expansion can use illegal types. Making expansion early allows
>      // legalizing these types prior to selection.
> diff --git a/llvm/test/CodeGen/X86/is_fpclass.ll b/llvm/test/CodeGen/X86/is_fpclass.ll
> index 6a531817e771..3b69011e3866 100644
> --- a/llvm/test/CodeGen/X86/is_fpclass.ll
> +++ b/llvm/test/CodeGen/X86/is_fpclass.ll
> @@ -5,18 +5,18 @@
>  define i1 @isnan_f(float %x) {
>  ; CHECK-32-LABEL: isnan_f:
>  ; CHECK-32:       # %bb.0: # %entry
> -; CHECK-32-NEXT:    flds {{[0-9]+}}(%esp)
> -; CHECK-32-NEXT:    fucomp %st(0)
> -; CHECK-32-NEXT:    fnstsw %ax
> -; CHECK-32-NEXT:    # kill: def $ah killed $ah killed $ax
> -; CHECK-32-NEXT:    sahf
> -; CHECK-32-NEXT:    setp %al
> +; CHECK-32-NEXT:    movl $2147483647, %eax # imm = 0x7FFFFFFF
> +; CHECK-32-NEXT:    andl {{[0-9]+}}(%esp), %eax
> +; CHECK-32-NEXT:    cmpl $2139095041, %eax # imm = 0x7F800001
> +; CHECK-32-NEXT:    setge %al
>  ; CHECK-32-NEXT:    retl
>  ;
>  ; CHECK-64-LABEL: isnan_f:
>  ; CHECK-64:       # %bb.0: # %entry
> -; CHECK-64-NEXT:    ucomiss %xmm0, %xmm0
> -; CHECK-64-NEXT:    setp %al
> +; CHECK-64-NEXT:    movd %xmm0, %eax
> +; CHECK-64-NEXT:    andl $2147483647, %eax # imm = 0x7FFFFFFF
> +; CHECK-64-NEXT:    cmpl $2139095041, %eax # imm = 0x7F800001
> +; CHECK-64-NEXT:    setge %al
>  ; CHECK-64-NEXT:    retq
>  entry:
>    %0 = tail call i1 @llvm.is.fpclass.f32(float %x, i32 3)  ; "nan"
> @@ -26,18 +26,18 @@ entry:
> ......
> ```
The expansion code should be considering strictfp on the parent function, not the node itself


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152993