[CIR][CIRGen] Support __builtin_isinf_sign

[PikachuHyA]

No description provided.

[bcardosolopes]

Firstly, it uses IsFPClass to determine if a value is infinite, whereas the original CGBuiltin implementation used direct comparisons with infinity constants.

I'm assuming IsFPClass will lead to the same final LLVM? If so LGTM

[bcardosolopes]

@orbiri fyi (you might want to track this too)

[orbiri]

Can you explain why can’t signbit return a Boolean? What is its other usages? (I don’t have full context so that would help me to understand better this patch 😇)

Looking around, it seems even that C++ is exposing this API with a Boolean. https://en.cppreference.com/w/cpp/numeric/math/signbit

Perhaps it is time to convert this op to Boolean as well? :)

[PikachuHyA]

Can you explain why can’t signbit return a Boolean? What is its other usages?

please see the error in #1187

[PikachuHyA]

Firstly, it uses IsFPClass to determine if a value is infinite, whereas the original CGBuiltin implementation used direct comparisons with infinity constants.

I'm assuming IsFPClass will lead to the same final LLVM?

If you are inquiring whether their semantics are equivalent, the answer is yes.

[orbiri]

Can you explain why can’t signbit return a Boolean? What is its other usages?

please see the error in #1187

This error indicates that there’s either an error in your codegen or in the lowering code. In either way, I would not recommend increasing the tech debt with this change.

I would recommend using the -debug flag of cir-opt and inspect the lowering step by step!

[bcardosolopes]

In either way, I would not recommend increasing the tech debt with this change.

Thanks @orbiri, way to go. Reviewed and landed #1187, @PikachuHyA let me know when this PR is ready again

[PikachuHyA]

@orbiri After updating SignBitOp to return bool, this patch still leads to two unnecessary type conversions: from i1 to i8 and then back to i1. I think it is related to #480 , please feel free to share your thoughts on this.

I left a comment in clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp:

// FIXME: CIR currently converts cir::BoolType to i8 type unconditionally.
// See https://github.com/llvm/clangir/issues/480
// Fixing this issue will eliminate redundant cast instructions
// for IsInf and IsNeg: i1 -> i8 -> i1

The LLVM IR generated by running:

./bin/clang ../clang/test/CIR/CodeGen/builtin-isinf-sign.c -Xclang -emit-llvm -o t.ll -c -fclangir

is as follows:

define dso_local i32 @test_float_isinf_sign(float %0) #0 {
  %2 = alloca float, i64 1, align 4
  %3 = alloca i32, i64 1, align 4
  store float %0, ptr %2, align 4
  %4 = load float, ptr %2, align 4
  %5 = call float @llvm.fabs.f32(float %4)
  %6 = call i1 @llvm.is.fpclass.f32(float %5, i32 516)
  %7 = zext i1 %6 to i8
  %8 = bitcast float %4 to i32
  %9 = icmp slt i32 %8, 0
  %10 = zext i1 %9 to i8
  %11 = trunc i8 %10 to i1
  %12 = select i1 %11, i32 -1, i32 1
  %13 = trunc i8 %7 to i1
  %14 = select i1 %13, i32 %12, i32 0
  store i32 %14, ptr %3, align 4
  %15 = load i32, ptr %3, align 4
  ret i32 %15
}

As shown, there are unnecessary conversions: %6 -> %7 -> %13 and %9 -> %10 -> %11.

Additionally, the LLVM IR generated by running:

./bin/clang ../clang/test/CIR/CodeGen/builtin-isinf-sign.c -Xclang -emit-llvm -o t.orig.ll -c

is:

define dso_local i32 @test_float_isinf_sign(float noundef %x) #0 {
entry:
  %x.addr = alloca float, align 4
  store float %x, ptr %x.addr, align 4
  %0 = load float, ptr %x.addr, align 4
  %1 = call float @llvm.fabs.f32(float %0) #2
  %isinf = fcmp oeq float %1, 0x7FF0000000000000
  %2 = bitcast float %0 to i32
  %3 = icmp slt i32 %2, 0
  %4 = select i1 %3, i32 -1, i32 1
  %5 = select i1 %isinf, i32 %4, i32 0
  ret i32 %5
}

[PikachuHyA]

@orbiri ping

[orbiri]

It will be sorted out very soon! Don’t optimize on the llvm output but rather on the CIR output :)