[flang][OpenMP] Fix missing missing inode issue #130798
Merged
+18
−10
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
mlir:llvm
mlir
flang
|
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-flang-fir-hlfir Author: Michael Klemm (mjklemm) ChangesWhen outlining an offload region, Flang creates a unique name by querying an inode ID. However, when the name of the actual source file does not match the logical file in a Full diff: https://github.com/llvm/llvm-project/pull/130798.diff 2 Files Affected:
diff --git a/flang/test/Lower/OpenMP/missing-inode.f90 b/flang/test/Lower/OpenMP/missing-inode.f90
new file mode 100644
index 0000000000000..01e662557cfe6
--- /dev/null
+++ b/flang/test/Lower/OpenMP/missing-inode.f90
@@ -0,0 +1,8 @@
+! RUN: %flang_fc1 -emit-llvm -fopenmp -fopenmp-version=51 -fopenmp-targets=amdgcn-amd-amdhsa -o - %s | FileCheck %s
+program missing_inode
+#line "/this_path_should_not_exist_on_any_system_out_there/and_if_it_does_it_will_break_the/tes.f90" 700
+ ! CHECK: define internal void @__omp_offloading_{{[0-9a-f]+}}_{{[0-9a-f]+}}__QQmain_l701
+ !$omp target
+ call some_routine()
+ !$omp end target
+end program missing_inode
\ No newline at end of file
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 3373f19a006ba..5e2879b6787ee 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -4284,7 +4284,7 @@ LogicalResult convertFlagsAttr(Operation *op, mlir::omp::FlagsAttr attribute,
return success();
}
-static bool getTargetEntryUniqueInfo(llvm::TargetRegionEntryInfo &targetInfo,
+static void getTargetEntryUniqueInfo(llvm::TargetRegionEntryInfo &targetInfo,
omp::TargetOp targetOp,
llvm::StringRef parentName = "") {
auto fileLoc = targetOp.getLoc()->findInstanceOf<FileLineColLoc>();
@@ -4293,15 +4293,16 @@ static bool getTargetEntryUniqueInfo(llvm::TargetRegionEntryInfo &targetInfo,
StringRef fileName = fileLoc.getFilename().getValue();
llvm::sys::fs::UniqueID id;
+ uint64_t line = fileLoc.getLine();
if (auto ec = llvm::sys::fs::getUniqueID(fileName, id)) {
- targetOp.emitError("Unable to get unique ID for file");
- return false;
+ size_t fileHash = llvm::hash_value(fileName.str());
+ size_t deviceId = 0xdeadf17e;
+ targetInfo =
+ llvm::TargetRegionEntryInfo(parentName, deviceId, fileHash, line);
+ } else {
+ targetInfo = llvm::TargetRegionEntryInfo(parentName, id.getDevice(),
+ id.getFile(), line);
}
-
- uint64_t line = fileLoc.getLine();
- targetInfo = llvm::TargetRegionEntryInfo(parentName, id.getDevice(),
- id.getFile(), line);
- return true;
}
static void
@@ -4901,8 +4902,7 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
llvm::TargetRegionEntryInfo entryInfo;
- if (!getTargetEntryUniqueInfo(entryInfo, targetOp, parentName))
- return failure();
+ getTargetEntryUniqueInfo(entryInfo, targetOp, parentName);
MapInfoData mapData;
collectMapDataFromMapOperands(mapData, mapVars, moduleTranslation, dl,
|
|
@llvm/pr-subscribers-mlir Author: Michael Klemm (mjklemm) ChangesWhen outlining an offload region, Flang creates a unique name by querying an inode ID. However, when the name of the actual source file does not match the logical file in a Full diff: https://github.com/llvm/llvm-project/pull/130798.diff 2 Files Affected:
diff --git a/flang/test/Lower/OpenMP/missing-inode.f90 b/flang/test/Lower/OpenMP/missing-inode.f90
new file mode 100644
index 0000000000000..01e662557cfe6
--- /dev/null
+++ b/flang/test/Lower/OpenMP/missing-inode.f90
@@ -0,0 +1,8 @@
+! RUN: %flang_fc1 -emit-llvm -fopenmp -fopenmp-version=51 -fopenmp-targets=amdgcn-amd-amdhsa -o - %s | FileCheck %s
+program missing_inode
+#line "/this_path_should_not_exist_on_any_system_out_there/and_if_it_does_it_will_break_the/tes.f90" 700
+ ! CHECK: define internal void @__omp_offloading_{{[0-9a-f]+}}_{{[0-9a-f]+}}__QQmain_l701
+ !$omp target
+ call some_routine()
+ !$omp end target
+end program missing_inode
\ No newline at end of file
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 3373f19a006ba..5e2879b6787ee 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -4284,7 +4284,7 @@ LogicalResult convertFlagsAttr(Operation *op, mlir::omp::FlagsAttr attribute,
return success();
}
-static bool getTargetEntryUniqueInfo(llvm::TargetRegionEntryInfo &targetInfo,
+static void getTargetEntryUniqueInfo(llvm::TargetRegionEntryInfo &targetInfo,
omp::TargetOp targetOp,
llvm::StringRef parentName = "") {
auto fileLoc = targetOp.getLoc()->findInstanceOf<FileLineColLoc>();
@@ -4293,15 +4293,16 @@ static bool getTargetEntryUniqueInfo(llvm::TargetRegionEntryInfo &targetInfo,
StringRef fileName = fileLoc.getFilename().getValue();
llvm::sys::fs::UniqueID id;
+ uint64_t line = fileLoc.getLine();
if (auto ec = llvm::sys::fs::getUniqueID(fileName, id)) {
- targetOp.emitError("Unable to get unique ID for file");
- return false;
+ size_t fileHash = llvm::hash_value(fileName.str());
+ size_t deviceId = 0xdeadf17e;
+ targetInfo =
+ llvm::TargetRegionEntryInfo(parentName, deviceId, fileHash, line);
+ } else {
+ targetInfo = llvm::TargetRegionEntryInfo(parentName, id.getDevice(),
+ id.getFile(), line);
}
-
- uint64_t line = fileLoc.getLine();
- targetInfo = llvm::TargetRegionEntryInfo(parentName, id.getDevice(),
- id.getFile(), line);
- return true;
}
static void
@@ -4901,8 +4902,7 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
llvm::TargetRegionEntryInfo entryInfo;
- if (!getTargetEntryUniqueInfo(entryInfo, targetOp, parentName))
- return failure();
+ getTargetEntryUniqueInfo(entryInfo, targetOp, parentName);
MapInfoData mapData;
collectMapDataFromMapOperands(mapData, mapVars, moduleTranslation, dl,
|
|
@llvm/pr-subscribers-mlir-openmp Author: Michael Klemm (mjklemm) ChangesWhen outlining an offload region, Flang creates a unique name by querying an inode ID. However, when the name of the actual source file does not match the logical file in a Full diff: https://github.com/llvm/llvm-project/pull/130798.diff 2 Files Affected:
diff --git a/flang/test/Lower/OpenMP/missing-inode.f90 b/flang/test/Lower/OpenMP/missing-inode.f90
new file mode 100644
index 0000000000000..01e662557cfe6
--- /dev/null
+++ b/flang/test/Lower/OpenMP/missing-inode.f90
@@ -0,0 +1,8 @@
+! RUN: %flang_fc1 -emit-llvm -fopenmp -fopenmp-version=51 -fopenmp-targets=amdgcn-amd-amdhsa -o - %s | FileCheck %s
+program missing_inode
+#line "/this_path_should_not_exist_on_any_system_out_there/and_if_it_does_it_will_break_the/tes.f90" 700
+ ! CHECK: define internal void @__omp_offloading_{{[0-9a-f]+}}_{{[0-9a-f]+}}__QQmain_l701
+ !$omp target
+ call some_routine()
+ !$omp end target
+end program missing_inode
\ No newline at end of file
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 3373f19a006ba..5e2879b6787ee 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -4284,7 +4284,7 @@ LogicalResult convertFlagsAttr(Operation *op, mlir::omp::FlagsAttr attribute,
return success();
}
-static bool getTargetEntryUniqueInfo(llvm::TargetRegionEntryInfo &targetInfo,
+static void getTargetEntryUniqueInfo(llvm::TargetRegionEntryInfo &targetInfo,
omp::TargetOp targetOp,
llvm::StringRef parentName = "") {
auto fileLoc = targetOp.getLoc()->findInstanceOf<FileLineColLoc>();
@@ -4293,15 +4293,16 @@ static bool getTargetEntryUniqueInfo(llvm::TargetRegionEntryInfo &targetInfo,
StringRef fileName = fileLoc.getFilename().getValue();
llvm::sys::fs::UniqueID id;
+ uint64_t line = fileLoc.getLine();
if (auto ec = llvm::sys::fs::getUniqueID(fileName, id)) {
- targetOp.emitError("Unable to get unique ID for file");
- return false;
+ size_t fileHash = llvm::hash_value(fileName.str());
+ size_t deviceId = 0xdeadf17e;
+ targetInfo =
+ llvm::TargetRegionEntryInfo(parentName, deviceId, fileHash, line);
+ } else {
+ targetInfo = llvm::TargetRegionEntryInfo(parentName, id.getDevice(),
+ id.getFile(), line);
}
-
- uint64_t line = fileLoc.getLine();
- targetInfo = llvm::TargetRegionEntryInfo(parentName, id.getDevice(),
- id.getFile(), line);
- return true;
}
static void
@@ -4901,8 +4902,7 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
llvm::TargetRegionEntryInfo entryInfo;
- if (!getTargetEntryUniqueInfo(entryInfo, targetOp, parentName))
- return failure();
+ getTargetEntryUniqueInfo(entryInfo, targetOp, parentName);
MapInfoData mapData;
collectMapDataFromMapOperands(mapData, mapVars, moduleTranslation, dl,
|
|
@llvm/pr-subscribers-mlir-llvm Author: Michael Klemm (mjklemm) ChangesWhen outlining an offload region, Flang creates a unique name by querying an inode ID. However, when the name of the actual source file does not match the logical file in a Full diff: https://github.com/llvm/llvm-project/pull/130798.diff 2 Files Affected:
diff --git a/flang/test/Lower/OpenMP/missing-inode.f90 b/flang/test/Lower/OpenMP/missing-inode.f90
new file mode 100644
index 0000000000000..01e662557cfe6
--- /dev/null
+++ b/flang/test/Lower/OpenMP/missing-inode.f90
@@ -0,0 +1,8 @@
+! RUN: %flang_fc1 -emit-llvm -fopenmp -fopenmp-version=51 -fopenmp-targets=amdgcn-amd-amdhsa -o - %s | FileCheck %s
+program missing_inode
+#line "/this_path_should_not_exist_on_any_system_out_there/and_if_it_does_it_will_break_the/tes.f90" 700
+ ! CHECK: define internal void @__omp_offloading_{{[0-9a-f]+}}_{{[0-9a-f]+}}__QQmain_l701
+ !$omp target
+ call some_routine()
+ !$omp end target
+end program missing_inode
\ No newline at end of file
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 3373f19a006ba..5e2879b6787ee 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -4284,7 +4284,7 @@ LogicalResult convertFlagsAttr(Operation *op, mlir::omp::FlagsAttr attribute,
return success();
}
-static bool getTargetEntryUniqueInfo(llvm::TargetRegionEntryInfo &targetInfo,
+static void getTargetEntryUniqueInfo(llvm::TargetRegionEntryInfo &targetInfo,
omp::TargetOp targetOp,
llvm::StringRef parentName = "") {
auto fileLoc = targetOp.getLoc()->findInstanceOf<FileLineColLoc>();
@@ -4293,15 +4293,16 @@ static bool getTargetEntryUniqueInfo(llvm::TargetRegionEntryInfo &targetInfo,
StringRef fileName = fileLoc.getFilename().getValue();
llvm::sys::fs::UniqueID id;
+ uint64_t line = fileLoc.getLine();
if (auto ec = llvm::sys::fs::getUniqueID(fileName, id)) {
- targetOp.emitError("Unable to get unique ID for file");
- return false;
+ size_t fileHash = llvm::hash_value(fileName.str());
+ size_t deviceId = 0xdeadf17e;
+ targetInfo =
+ llvm::TargetRegionEntryInfo(parentName, deviceId, fileHash, line);
+ } else {
+ targetInfo = llvm::TargetRegionEntryInfo(parentName, id.getDevice(),
+ id.getFile(), line);
}
-
- uint64_t line = fileLoc.getLine();
- targetInfo = llvm::TargetRegionEntryInfo(parentName, id.getDevice(),
- id.getFile(), line);
- return true;
}
static void
@@ -4901,8 +4902,7 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
llvm::TargetRegionEntryInfo entryInfo;
- if (!getTargetEntryUniqueInfo(entryInfo, targetOp, parentName))
- return failure();
+ getTargetEntryUniqueInfo(entryInfo, targetOp, parentName);
MapInfoData mapData;
collectMapDataFromMapOperands(mapData, mapVars, moduleTranslation, dl,
|
kparzysz
approved these changes
Mar 11, 2025
skatrak
approved these changes
Mar 12, 2025
There was a problem hiding this comment.
Thank you, it looks like this would hopefully allow us to translate split MLIR files (i.e. --split-input-file) and code passed from stdin containing target regions as well.
Perhaps you could propose this fix for clang, since it has a similar issue: #62099.
frederik-h
pushed a commit
to frederik-h/llvm-project
that referenced
this pull request
Mar 18, 2025
When outlining an offload region, Flang creates a unique name by querying an inode ID. However, when the name of the actual source file does not match the logical file in a `#line` preprocessor directive, code-gen was failing as it could not determine the inode ID. This PR checks for this condition and if the logical file name does not exist, the inode is replaced with a hash value created from the source code itself.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When outlining an offload region, Flang creates a unique name by querying an inode ID. However, when the name of the actual source file does not match the logical file in a
#linepreprocessor directive, code-gen was failing as it could not determine the inode ID. This PR checks for this condition and if the logical file name does not exist, the inode is replaced with a hash value created from the source code itself.