Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
627bcb3
Add CIR sqrt builtin support for X86
Priyanshu3820 Nov 26, 2025
d095f5a
Merge branch 'main' into cir-sqrtps-impl
Priyanshu3820 Nov 26, 2025
8c13c6f
Merge branch 'main' into cir-sqrtps-impl
Priyanshu3820 Nov 27, 2025
17a3e66
Merge branch 'main' into cir-sqrtps-impl
Priyanshu3820 Nov 27, 2025
01bb815
Merge branch 'main' into cir-sqrtps-impl
Priyanshu3820 Nov 28, 2025
4a39fd7
Implement sqrt builtins for all vector sizes
Priyanshu3820 Nov 29, 2025
ef3fd97
Test file renamed
Priyanshu3820 Nov 30, 2025
9705673
Add sqrt changes patch
Priyanshu3820 Dec 2, 2025
21119e5
group with other floating point ops
Priyanshu3820 Dec 3, 2025
90878ec
place the implementation with other floating point ops
Priyanshu3820 Dec 3, 2025
3529f40
Update clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
Priyanshu3820 Dec 3, 2025
92d0ac3
Update clang/lib/CIR/CodeGen/CIRGenBuiltinX86.cpp
Priyanshu3820 Dec 3, 2025
0385662
Update clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
Priyanshu3820 Dec 3, 2025
ddcb7b8
update clang\lib\CIR\Lowering\DirectToLLVM\LowerToLLVM.cpp
Priyanshu3820 Dec 3, 2025
1e846e7
update clang\lib\CIR\Lowering\DirectToLLVM\LowerToLLVM.cpp
Priyanshu3820 Dec 3, 2025
233efad
Update clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
Priyanshu3820 Dec 3, 2025
9d940bc
Update clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
Priyanshu3820 Dec 3, 2025
51bbcca
Remove BOM character
Priyanshu3820 Dec 4, 2025
f901f03
Merge branch 'cir-sqrtps-impl' of https://github.com/Priyanshu3820/ll…
Priyanshu3820 Dec 4, 2025
e5789b6
Apply suggestion from @Copilot
Priyanshu3820 Dec 4, 2025
8937b12
Apply suggestion from @andykaylor
Priyanshu3820 Dec 4, 2025
8a02c50
add description
Priyanshu3820 Dec 4, 2025
9923a62
Resolve merge conflict in LowerToLLVM.cpp
Priyanshu3820 Dec 4, 2025
82a9395
Remove undefined sqrt builtin cases
Priyanshu3820 Dec 4, 2025
6bd3282
Remove unused getLLVMIntrinsicNameForType function
Priyanshu3820 Dec 4, 2025
8232ce8
Removed braces
Priyanshu3820 Dec 4, 2025
bc8e4cc
Update clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
Priyanshu3820 Dec 4, 2025
9284761
update clang\lib\CIR\Lowering\DirectToLLVM\LowerToLLVM.h
Priyanshu3820 Dec 4, 2025
8647b5c
Update clang/test/CIR/CodeGen/X86/cir-sqrt-builtins.c
Priyanshu3820 Dec 4, 2025
4bac65a
Update test
Priyanshu3820 Dec 4, 2025
b1ff2ab
update clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
Priyanshu3820 Dec 4, 2025
8843006
Remove unused include
Priyanshu3820 Dec 5, 2025
ed82423
Move sqrt lowering with other floating point operations
Priyanshu3820 Dec 5, 2025
9e8bec2
Merge branch 'main' into cir-sqrtps-impl
Priyanshu3820 Dec 5, 2025
961c9f9
Remove BOM character
Priyanshu3820 Dec 6, 2025
e5d1a6d
gergeebranch 'cir-sqrtps-impl' of https://github.com/Priyanshu3820/ll…
Priyanshu3820 Dec 6, 2025
44ddd79
Merge branch 'main' into cir-sqrtps-impl
Priyanshu3820 Dec 6, 2025
4dd8aa0
Delete my-sqrt-changes.patch
Priyanshu3820 Dec 6, 2025
cc5ffa1
Update errorNYI call
Priyanshu3820 Dec 7, 2025
6d43c43
Merge branch 'cir-sqrtps-impl' of https://github.com/Priyanshu3820/ll…
Priyanshu3820 Dec 7, 2025
47f9b2f
update clang\lib\CIR\Lowering\DirectToLLVM\LowerToLLVM.cpp
Priyanshu3820 Dec 7, 2025
15f1f4f
update clang/test/CIR/CodeGen/X86/cir-sqrt-builtins.c
Priyanshu3820 Dec 7, 2025
b12779a
Fix line endings in CIRGenBuiltinX86.cpp
Priyanshu3820 Dec 8, 2025
996b7e7
Fix formatting
andykaylor Dec 8, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions clang/include/clang/CIR/Dialect/IR/CIROps.td
Original file line number Diff line number Diff line change
Expand Up @@ -4642,6 +4642,14 @@ class CIR_UnaryFPToFPBuiltinOp<string mnemonic, string llvmOpName>
let llvmOp = llvmOpName;
}

def CIR_SqrtOp : CIR_UnaryFPToFPBuiltinOp<"sqrt", "SqrtOp"> {
let summary = "Floating-point square root operation";

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a description with an example of the CIR format?

We're trying to standardize the descriptions of all our operations to include an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had done that initially but removed the example after seeing other floating point ops didn't have examples too. Added it again now.

let description = [{
Computes the square root of a floating-point value or vector.
}];
}

def CIR_ACosOp : CIR_UnaryFPToFPBuiltinOp<"acos", "ACosOp"> {
let summary = "Computes the arcus cosine of the specified value";
let description = [{
Expand Down
14 changes: 13 additions & 1 deletion clang/lib/CIR/CodeGen/CIRGenBuiltinX86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -781,9 +781,21 @@ mlir::Value CIRGenFunction::emitX86BuiltinExpr(unsigned builtinID,
case X86::BI__builtin_ia32_sqrtsh_round_mask:
case X86::BI__builtin_ia32_sqrtsd_round_mask:
case X86::BI__builtin_ia32_sqrtss_round_mask:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to insert a call to errorNYI here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I just move the sqrt implementation to the top? That way, the builtins that are currently above them will just fall through to the NYI error at the end.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. We are attempting to keep builtins in the same relative order as they appear in classic codegen and the incubator in order to make the code easier to navigate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood!

case X86::BI__builtin_ia32_sqrtpd256:
case X86::BI__builtin_ia32_sqrtpd:
case X86::BI__builtin_ia32_sqrtps256:
case X86::BI__builtin_ia32_sqrtps:
case X86::BI__builtin_ia32_sqrtph256:
case X86::BI__builtin_ia32_sqrtph:
errorNYI("Unimplemented builtin");
return {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should remove these. It doesn't look like these will even be defined in the latest top of trunk code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

case X86::BI__builtin_ia32_sqrtph512:
case X86::BI__builtin_ia32_sqrtps512:
case X86::BI__builtin_ia32_sqrtpd512:
case X86::BI__builtin_ia32_sqrtpd512: {
mlir::Location loc = getLoc(expr->getExprLoc());
mlir::Value arg = ops[0];
return builder.create<cir::SqrtOp>(loc, arg.getType(), arg).getResult();
}
case X86::BI__builtin_ia32_pmuludq128:
case X86::BI__builtin_ia32_pmuludq256:
case X86::BI__builtin_ia32_pmuludq512:
Expand Down
90 changes: 89 additions & 1 deletion clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//====- LowerToLLVM.cpp - Lowering from CIR to LLVMIR ---------------------===//
//====- LowerToLLVM.cpp - Lowering from CIR to LLVMIR ---------------------===//
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What changed on this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like a phantom diff caused due to line ending differences.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The line endings may be automatically updated to match the repository settings when the changes are committed, but it's a good idea to avoid changing line ending styles. There are a few lit tests where the line ending kind is actually important.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still need to remove whatever BOM character is on this line.

//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
Expand Down Expand Up @@ -30,6 +30,7 @@
#include "mlir/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.h"
#include "mlir/Target/LLVMIR/Export.h"
#include "mlir/Transforms/DialectConversion.h"
#include "clang/Basic/LLVM.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed. Thanks for pointing this out. The file only uses MLIR types which are already included via the other headers. I'll remove it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can still be removed, right?

#include "clang/CIR/Dialect/IR/CIRAttrs.h"
#include "clang/CIR/Dialect/IR/CIRDialect.h"
#include "clang/CIR/Dialect/IR/CIRTypes.h"
Expand All @@ -45,6 +46,93 @@
using namespace cir;
using namespace llvm;


static std::string getLLVMIntrinsicNameForType(mlir::Type llvmTy) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't being called. You can remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it! done.

std::string s;
{
llvm::raw_string_ostream os(s);
os << llvmTy;
}
return s;
}

// Actual lowering
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing the beginning of the function, and this should be sunk down to be located near the other floating point operations,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must have somehow removed the beginning during some commits. It is now rectified, moved to where other floating point ops are located and #include "clang/Basic/LLVM.h" is removed too.

mlir::LogicalResult CIRToLLVMSqrtOpLowering::matchAndRewrite(
cir::SqrtOp op, typename cir::SqrtOp::Adaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const {

mlir::Location loc = op.getLoc();
mlir::MLIRContext *ctx = rewriter.getContext();

mlir::Type cirResTy = op.getResult().getType();
mlir::Type llvmResTy = getTypeConverter()->convertType(cirResTy);
if (!llvmResTy)
return op.emitOpError(
"expected LLVM dialect result type for cir.sqrt lowering");

Value operand = adaptor.getInput();
Value llvmOperand = operand;
if (operand.getType() != llvmResTy) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed!

llvmOperand = rewriter.create<LLVM::BitcastOp>(loc, llvmResTy, operand);
}

// Build the llvm.sqrt.* intrinsic name depending on scalar vs vector result
std::string intrinsicName = "llvm.sqrt.";
std::string suffix;

// If the CIR result type is a vector, include the 'vN' part in the suffix.
if (auto vec = cirResTy.dyn_cast<cir::VectorType>()) {
Type elt = vec.getElementType();
if (auto f = elt.dyn_cast<cir::FloatType>()) {
unsigned width = f.getWidth();
unsigned n = vec.getNumElements();
if (width == 32)
suffix = "v" + std::to_string(n) + "f32";
else if (width == 64)
suffix = "v" + std::to_string(n) + "f64";
else if (width == 16)
suffix = "v" + std::to_string(n) + "f16";
else
return op.emitOpError("unsupported float width for sqrt");
} else {
return op.emitOpError("vector element must be floating point for sqrt");
}
} else if (auto f = cirResTy.dyn_cast<cir::FloatType>()) {
// Scalar float
unsigned width = f.getWidth();
if (width == 32)
suffix = "f32";
else if (width == 64)
suffix = "f64";
else if (width == 16)
suffix = "f16";
else
return op.emitOpError("unsupported float width for sqrt");
} else {
return op.emitOpError("unsupported type for cir.sqrt lowering");
}

intrinsicName += suffix;

// Ensure the llvm intrinsic function exists at module scope. Insert it at
// the start of the module body using an insertion guard.
ModuleOp module = op->getParentOfType<ModuleOp>();
if (!module.lookupSymbol<LLVM::LLVMFuncOp>(intrinsicName)) {
OpBuilder::InsertionGuard guard(rewriter);
rewriter.setInsertionPointToStart(module.getBody());
auto llvmFnType = LLVM::LLVMFunctionType::get(ctx, llvmResTy, {llvmResTy},
/*isVarArg=*/false);
rewriter.create<LLVM::LLVMFuncOp>(loc, intrinsicName, llvmFnType);
}

// Create the call and replace cir.sqrt
auto callee = SymbolRefAttr::get(ctx, intrinsicName);
rewriter.replaceOpWithNewOp<LLVM::CallOp>(op, llvmResTy, callee,
ArrayRef<Value>{llvmOperand});

return mlir::success();
}

namespace cir {
namespace direct {

Expand Down
13 changes: 13 additions & 0 deletions clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,19 @@
#include "mlir/Transforms/DialectConversion.h"
#include "clang/CIR/Dialect/IR/CIRDialect.h"

namespace cir {
class SqrtOp;
}

class CIRToLLVMSqrtOpLowering : public mlir::OpConversionPattern<cir::SqrtOp> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't need this. In the upstream CIR implementation, these definitions are automatically generated by TableGen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

public:
using mlir::OpConversionPattern<cir::SqrtOp>::OpConversionPattern;

mlir::LogicalResult
matchAndRewrite(cir::SqrtOp op, typename cir::SqrtOp::Adaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const override;
};

namespace cir {

namespace direct {
Expand Down
29 changes: 29 additions & 0 deletions clang/test/CIR/CodeGen/X86/cir-sqrt-builtins.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#include <immintrin.h>
// Test X86-specific sqrt builtins

// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
// RUN: FileCheck --input-file=%t.cir %s

// Test __builtin_ia32_sqrtph512
__m512h test_sqrtph512(__m512h a) {
return __builtin_ia32_sqrtph512(a);
}
// CHECK: cir.func @test_sqrtph512
// CHECK: [[RES:%.*]] = cir.sqrt {{%.*}} : !cir.vector<!cir.fp16 x 32>
// CHECK: cir.return [[RES]]

// Test __builtin_ia32_sqrtps512
__m512 test_sqrtps512(__m512 a) {
return __builtin_ia32_sqrtps512(a);
}
// CHECK: cir.func @test_sqrtps512
// CHECK: [[RES:%.*]] = cir.sqrt {{%.*}} : !cir.vector<!cir.float x 16>
// CHECK: cir.return [[RES]]

// Test __builtin_ia32_sqrtpd512
__m512d test_sqrtpd512(__m512d a) {
return __builtin_ia32_sqrtpd512(a);
}
// CHECK: cir.func @test_sqrtpd512
// CHECK: [[RES:%.*]] = cir.sqrt {{%.*}} : !cir.vector<!cir.double x 8>
// CHECK: cir.return [[RES]]
Binary file added my-sqrt-changes.patch
Binary file not shown.