[clang] [cir-translate] Fix crash issue where the data layout string is missing (PR #147209)

2025-07-09 Thread via cfe-commits 

https://github.com/darkbuck closed 
https://github.com/llvm/llvm-project/pull/147209
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [cir-translate] Fix crash issue where the data layout string is missing (PR #147209)

2025-07-09 Thread Bruno Cardoso Lopes via cfe-commits 

https://github.com/bcardosolopes approved this pull request.

LGTM pending my last comment

https://github.com/llvm/llvm-project/pull/147209
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [cir-translate] Fix crash issue where the data layout string is missing (PR #147209)

2025-07-09 Thread Bruno Cardoso Lopes via cfe-commits 


@@ -82,12 +85,17 @@ llvm::LogicalResult 
prepareCIRModuleDataLayout(mlir::ModuleOp mod,
 
   // Data layout is fully determined by the target triple. Here we only pass 
the
   // triple to get the data layout.
+  llvm::IntrusiveRefCntPtr diagID(
+  new clang::DiagnosticIDs);
+  clang::DiagnosticOptions diagOpts;
+  clang::DiagnosticsEngine diagnostics(diagID, diagOpts,
+   new clang::IgnoringDiagConsumer());
   llvm::Triple triple(rawTriple);
   clang::TargetOptions targetOptions;
   targetOptions.Triple = rawTriple;
   // FIXME: AllocateTarget is a big deal. Better make it a global state.
-  std::unique_ptr targetInfo =
-  clang::targets::AllocateTarget(llvm::Triple(rawTriple), targetOptions);
+  llvm::IntrusiveRefCntPtr targetInfo =
+  clang::TargetInfo::CreateTargetInfo(diagnostics, targetOptions);

bcardosolopes wrote:

I'm fine with this PR as is if you can put a follow up PR that tries adding the 
`create*` variant mentioned above! 

https://github.com/llvm/llvm-project/pull/147209
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [cir-translate] Fix crash issue where the data layout string is missing (PR #147209)

2025-07-09 Thread Bruno Cardoso Lopes via cfe-commits 


@@ -82,12 +85,17 @@ llvm::LogicalResult 
prepareCIRModuleDataLayout(mlir::ModuleOp mod,
 
   // Data layout is fully determined by the target triple. Here we only pass 
the
   // triple to get the data layout.
+  llvm::IntrusiveRefCntPtr diagID(
+  new clang::DiagnosticIDs);
+  clang::DiagnosticOptions diagOpts;
+  clang::DiagnosticsEngine diagnostics(diagID, diagOpts,
+   new clang::IgnoringDiagConsumer());
   llvm::Triple triple(rawTriple);
   clang::TargetOptions targetOptions;
   targetOptions.Triple = rawTriple;
   // FIXME: AllocateTarget is a big deal. Better make it a global state.
-  std::unique_ptr targetInfo =
-  clang::targets::AllocateTarget(llvm::Triple(rawTriple), targetOptions);
+  llvm::IntrusiveRefCntPtr targetInfo =
+  clang::TargetInfo::CreateTargetInfo(diagnostics, targetOptions);

bcardosolopes wrote:

re `IntrusiveRefCntPtr`: yea, I see other examples around now.

https://github.com/llvm/llvm-project/pull/147209
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [cir-translate] Fix crash issue where the data layout string is missing (PR #147209)

2025-07-07 Thread Andy Kaylor via cfe-commits 


@@ -82,12 +85,17 @@ llvm::LogicalResult 
prepareCIRModuleDataLayout(mlir::ModuleOp mod,
 
   // Data layout is fully determined by the target triple. Here we only pass 
the
   // triple to get the data layout.
+  llvm::IntrusiveRefCntPtr diagID(
+  new clang::DiagnosticIDs);
+  clang::DiagnosticOptions diagOpts;
+  clang::DiagnosticsEngine diagnostics(diagID, diagOpts,
+   new clang::IgnoringDiagConsumer());
   llvm::Triple triple(rawTriple);
   clang::TargetOptions targetOptions;
   targetOptions.Triple = rawTriple;
   // FIXME: AllocateTarget is a big deal. Better make it a global state.

andykaylor wrote:

Can you update this comment to just refer to TargetOptions? It seems that we're 
only using the data layout string here, but there are many other things in 
TargetOptions that might be relevant at some point.

https://github.com/llvm/llvm-project/pull/147209
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [cir-translate] Fix crash issue where the data layout string is missing (PR #147209)

2025-07-07 Thread Andy Kaylor via cfe-commits 


@@ -82,12 +85,17 @@ llvm::LogicalResult 
prepareCIRModuleDataLayout(mlir::ModuleOp mod,
 
   // Data layout is fully determined by the target triple. Here we only pass 
the
   // triple to get the data layout.
+  llvm::IntrusiveRefCntPtr diagID(
+  new clang::DiagnosticIDs);
+  clang::DiagnosticOptions diagOpts;
+  clang::DiagnosticsEngine diagnostics(diagID, diagOpts,
+   new clang::IgnoringDiagConsumer());
   llvm::Triple triple(rawTriple);
   clang::TargetOptions targetOptions;
   targetOptions.Triple = rawTriple;
   // FIXME: AllocateTarget is a big deal. Better make it a global state.
-  std::unique_ptr targetInfo =
-  clang::targets::AllocateTarget(llvm::Triple(rawTriple), targetOptions);
+  llvm::IntrusiveRefCntPtr targetInfo =
+  clang::TargetInfo::CreateTargetInfo(diagnostics, targetOptions);

andykaylor wrote:

Maybe we could add one? It seems there are multiple other callers that 
similarly don't use these.

https://github.com/llvm/llvm-project/pull/147209
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [cir-translate] Fix crash issue where the data layout string is missing (PR #147209)

2025-07-07 Thread Henrich Lauko via cfe-commits 

https://github.com/xlauko deleted 
https://github.com/llvm/llvm-project/pull/147209
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [cir-translate] Fix crash issue where the data layout string is missing (PR #147209)

2025-07-07 Thread Henrich Lauko via cfe-commits 


@@ -82,12 +85,17 @@ llvm::LogicalResult 
prepareCIRModuleDataLayout(mlir::ModuleOp mod,
 
   // Data layout is fully determined by the target triple. Here we only pass 
the
   // triple to get the data layout.
+  llvm::IntrusiveRefCntPtr diagID(
+  new clang::DiagnosticIDs);
+  clang::DiagnosticOptions diagOpts;
+  clang::DiagnosticsEngine diagnostics(diagID, diagOpts,
+   new clang::IgnoringDiagConsumer());
   llvm::Triple triple(rawTriple);
   clang::TargetOptions targetOptions;
   targetOptions.Triple = rawTriple;
   // FIXME: AllocateTarget is a big deal. Better make it a global state.

xlauko wrote:

Is this comment still relevant after the change?

https://github.com/llvm/llvm-project/pull/147209
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [cir-translate] Fix crash issue where the data layout string is missing (PR #147209)

2025-07-06 Thread via cfe-commits 

llvmbot wrote:



@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clangir

Author: None (darkbuck)


Changes

- Targets like 'aarch64' or 'arm' only populate the data layout string after 
the constructor. Need to call 'CreateTargetInfo' to setup them properly.

---
Full diff: https://github.com/llvm/llvm-project/pull/147209.diff


2 Files Affected:

- (modified) clang/test/CIR/Lowering/select.cir (+2-2) 
- (modified) clang/tools/cir-translate/cir-translate.cpp (+10-2) 


``diff
diff --git a/clang/test/CIR/Lowering/select.cir 
b/clang/test/CIR/Lowering/select.cir
index 71ca79a390e8b..37f9789e6b0f6 100644
--- a/clang/test/CIR/Lowering/select.cir
+++ b/clang/test/CIR/Lowering/select.cir
@@ -1,5 +1,5 @@
-// RUN: cir-translate -cir-to-llvmir --disable-cc-lowering -o %t.ll %s
-// RUN: FileCheck --input-file=%t.ll -check-prefix=LLVM %s
+// RUN: cir-translate -cir-to-llvmir --disable-cc-lowering -o - %s | FileCheck 
-check-prefix=LLVM %s
+// RUN: cir-translate -target aarch64 -cir-to-llvmir --disable-cc-lowering -o 
- %s | FileCheck -check-prefix=LLVM %s
 
 !s32i = !cir.int
 
diff --git a/clang/tools/cir-translate/cir-translate.cpp 
b/clang/tools/cir-translate/cir-translate.cpp
index e0e9414602a16..06bb31f677880 100644
--- a/clang/tools/cir-translate/cir-translate.cpp
+++ b/clang/tools/cir-translate/cir-translate.cpp
@@ -25,6 +25,9 @@
 #include "llvm/IR/Module.h"
 #include "llvm/TargetParser/Host.h"
 
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CIR/Dialect/IR/CIRDialect.h"
 #include "clang/CIR/Dialect/Passes.h"
@@ -82,12 +85,17 @@ llvm::LogicalResult 
prepareCIRModuleDataLayout(mlir::ModuleOp mod,
 
   // Data layout is fully determined by the target triple. Here we only pass 
the
   // triple to get the data layout.
+  llvm::IntrusiveRefCntPtr diagID(
+  new clang::DiagnosticIDs);
+  clang::DiagnosticOptions diagOpts;
+  clang::DiagnosticsEngine diagnostics(diagID, diagOpts,
+   new clang::IgnoringDiagConsumer());
   llvm::Triple triple(rawTriple);
   clang::TargetOptions targetOptions;
   targetOptions.Triple = rawTriple;
   // FIXME: AllocateTarget is a big deal. Better make it a global state.
-  std::unique_ptr targetInfo =
-  clang::targets::AllocateTarget(llvm::Triple(rawTriple), targetOptions);
+  llvm::IntrusiveRefCntPtr targetInfo =
+  clang::TargetInfo::CreateTargetInfo(diagnostics, targetOptions);
   if (!targetInfo) {
 mod.emitError() << "error: invalid target triple '" << rawTriple << "'\n";
 return llvm::failure();

``




https://github.com/llvm/llvm-project/pull/147209
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [cir-translate] Fix crash issue where the data layout string is missing (PR #147209)

2025-07-06 Thread via cfe-commits 

https://github.com/darkbuck created 
https://github.com/llvm/llvm-project/pull/147209

- Targets like 'aarch64' or 'arm' only populate the data layout string after 
the constructor. Need to call 'CreateTargetInfo' to setup them properly.

>From d436eb224999ee8eb6282ed000793db4e61b7017 Mon Sep 17 00:00:00 2001
From: Michael Liao 
Date: Sat, 5 Jul 2025 14:32:55 -0400
Subject: [PATCH] [cir-translate] Fix crash issue where the data layout string
 is missing

- Targets like 'aarch64' or 'arm' only populate the data layout string
  after the constructor. Need to call 'CreateTargetInfo' to setup them
  properly.
---
 clang/test/CIR/Lowering/select.cir  |  4 ++--
 clang/tools/cir-translate/cir-translate.cpp | 12 ++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/clang/test/CIR/Lowering/select.cir 
b/clang/test/CIR/Lowering/select.cir
index 71ca79a390e8b..37f9789e6b0f6 100644
--- a/clang/test/CIR/Lowering/select.cir
+++ b/clang/test/CIR/Lowering/select.cir
@@ -1,5 +1,5 @@
-// RUN: cir-translate -cir-to-llvmir --disable-cc-lowering -o %t.ll %s
-// RUN: FileCheck --input-file=%t.ll -check-prefix=LLVM %s
+// RUN: cir-translate -cir-to-llvmir --disable-cc-lowering -o - %s | FileCheck 
-check-prefix=LLVM %s
+// RUN: cir-translate -target aarch64 -cir-to-llvmir --disable-cc-lowering -o 
- %s | FileCheck -check-prefix=LLVM %s
 
 !s32i = !cir.int
 
diff --git a/clang/tools/cir-translate/cir-translate.cpp 
b/clang/tools/cir-translate/cir-translate.cpp
index e0e9414602a16..06bb31f677880 100644
--- a/clang/tools/cir-translate/cir-translate.cpp
+++ b/clang/tools/cir-translate/cir-translate.cpp
@@ -25,6 +25,9 @@
 #include "llvm/IR/Module.h"
 #include "llvm/TargetParser/Host.h"
 
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CIR/Dialect/IR/CIRDialect.h"
 #include "clang/CIR/Dialect/Passes.h"
@@ -82,12 +85,17 @@ llvm::LogicalResult 
prepareCIRModuleDataLayout(mlir::ModuleOp mod,
 
   // Data layout is fully determined by the target triple. Here we only pass 
the
   // triple to get the data layout.
+  llvm::IntrusiveRefCntPtr diagID(
+  new clang::DiagnosticIDs);
+  clang::DiagnosticOptions diagOpts;
+  clang::DiagnosticsEngine diagnostics(diagID, diagOpts,
+   new clang::IgnoringDiagConsumer());
   llvm::Triple triple(rawTriple);
   clang::TargetOptions targetOptions;
   targetOptions.Triple = rawTriple;
   // FIXME: AllocateTarget is a big deal. Better make it a global state.
-  std::unique_ptr targetInfo =
-  clang::targets::AllocateTarget(llvm::Triple(rawTriple), targetOptions);
+  llvm::IntrusiveRefCntPtr targetInfo =
+  clang::TargetInfo::CreateTargetInfo(diagnostics, targetOptions);
   if (!targetInfo) {
 mod.emitError() << "error: invalid target triple '" << rawTriple << "'\n";
 return llvm::failure();

___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits