[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Dominik Adamski via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbaca3c150733: Move SIMD alignment calculation to LLVM 
Frontend (authored by domada).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141910

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/CMakeLists.txt
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -3053,6 +3053,23 @@
   Builder.CreateBr(NewBlocks.front());
 }
 
+unsigned
+OpenMPIRBuilder::getOpenMPDefaultSimdAlign(const Triple &TargetTriple,
+   const StringMap &Features) {
+  if (TargetTriple.isX86()) {
+if (Features.lookup("avx512f"))
+  return 512;
+else if (Features.lookup("avx"))
+  return 256;
+return 128;
+  }
+  if (TargetTriple.isPPC())
+return 128;
+  if (TargetTriple.isWasm())
+return 128;
+  return 0;
+}
+
 void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop,
 MapVector AlignedVars,
 Value *IfCond, OrderKind Order,
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -502,6 +502,13 @@
ArrayRef Loops,
InsertPointTy ComputeIP);
 
+  /// Get the default alignment value for given target
+  ///
+  /// \param TargetTriple   Target triple
+  /// \param Features   StringMap which describes extra CPU features
+  static unsigned getOpenMPDefaultSimdAlign(const Triple &TargetTriple,
+const StringMap &Features);
+
 private:
   /// Modifies the canonical loop to be a statically-scheduled workshare loop.
   ///
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -500,8 +500,6 @@
   auto target_info = TargetInfo::CreateTargetInfo(
   m_compiler->getDiagnostics(), m_compiler->getInvocation().TargetOpts);
   if (log) {
-LLDB_LOGF(log, "Using SIMD alignment: %d",
-  target_info->getSimdDefaultAlign());
 LLDB_LOGF(log, "Target datalayout string: '%s'",
   target_info->getDataLayoutString());
 LLDB_LOGF(log, "Target ABI: '%s'", target_info->getABI().str().c_str());
Index: clang/lib/Basic/Targets/X86.cpp
===
--- clang/lib/Basic/Targets/X86.cpp
+++ clang/lib/Basic/Targets/X86.cpp
@@ -400,9 +400,6 @@
 return false;
   }
 
-  SimdDefaultAlign =
-  hasFeature("avx512f") ? 512 : hasFeature("avx") ? 256 : 128;
-
   // FIXME: We should allow long double type on 32-bits to match with GCC.
   // This requires backend to be able to lower f80 without x87 first.
   if (!HasX87 && LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -49,7 +49,6 @@
 SuitableAlign = 128;
 LargeArrayMinWidth = 128;
 LargeArrayAlign = 128;
-SimdDefaultAlign = 128;
 SigAtomicType = SignedLong;
 LongDoubleWidth = LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::IEEEquad();
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -87,7 +87,6 @@
   PPCTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple) {
 SuitableAlign = 128;
-SimdDefaultAlign = 128;
 LongDoubleWidth = LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::PPCDoubleDouble();
 HasStrictFP = true;
Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -119,7 +119,6 @@
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 0;
   MaxVectorAlign = 0;
   MaxTLSAlign = 0;
-  SimdDefaultAlign = 0;
   SizeType = UnsignedLong;
   PtrDiffType = S

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Kiran Chandramohan via Phabricator via lldb-commits]

kiranchandramohan accepted this revision.
kiranchandramohan added a comment.

LG.


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

https://reviews.llvm.org/D141910

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Argyrios Kyrtzidis via Phabricator via lldb-commits]

akyrtzi added a comment.

In D141910#4112144 , @domada wrote:

> @akyrtzi Thank you for your feedback. Can I land the patch?

Fine be me.


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

https://reviews.llvm.org/D141910

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Dominik Adamski via Phabricator via lldb-commits]

domada added a comment.

In D141910#4112164 , @tschuett wrote:

> For AArch64 the default alignment is 0? I would have expected 128.

The refactored function `TargetInfo::getSimdDefaultAlign` is used only for 
calculation of default alignment for `#pragma omp simd aligned(A)`. If user 
does not specify any alignment in OpenMP simd pragma, then we will assume that 
the alignment of `A` is equal to 128 for PPC, WebAssembly or for some X86 
targets and we insert proper assumptions into LLVM IR (please refer to: clang 
test for more details 
.
 For other targets like AArch64 we don't insert any assumptions into LLVM IR 
code.

I just refactored the code and I return the same values as previous function.


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

https://reviews.llvm.org/D141910

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Thorsten via Phabricator via lldb-commits]

tschuett added a comment.

For AArch64 the default alignment is 0? I would have expected 128.


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

https://reviews.llvm.org/D141910

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Dominik Adamski via Phabricator via lldb-commits]

domada added a comment.

@akyrtzi Thank you for your feedback. Can I land the patch?


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

https://reviews.llvm.org/D141910

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Argyrios Kyrtzidis via Phabricator via lldb-commits]

akyrtzi added a comment.

In D141910#4111048 , @domada wrote:

> Added changes in `clang/lib/AST/CMakeLists.txt` to address build issue 
> reported by @akyrtzi .
>
> I modified CMakeLists.txt so that it requires generation of missing 
> `Attributes.inc`.
>
> @akyrtzi  Please let me know if it solves your issue

This fixes the build issue 👍, thank you!


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

https://reviews.llvm.org/D141910

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Dominik Adamski via Phabricator via lldb-commits]

domada updated this revision to Diff 495627.
domada added a comment.

Patch rebased


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

https://reviews.llvm.org/D141910

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/CMakeLists.txt
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -3053,6 +3053,23 @@
   Builder.CreateBr(NewBlocks.front());
 }
 
+unsigned
+OpenMPIRBuilder::getOpenMPDefaultSimdAlign(const Triple &TargetTriple,
+   const StringMap &Features) {
+  if (TargetTriple.isX86()) {
+if (Features.lookup("avx512f"))
+  return 512;
+else if (Features.lookup("avx"))
+  return 256;
+return 128;
+  }
+  if (TargetTriple.isPPC())
+return 128;
+  if (TargetTriple.isWasm())
+return 128;
+  return 0;
+}
+
 void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop,
 MapVector AlignedVars,
 Value *IfCond, OrderKind Order,
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -502,6 +502,13 @@
ArrayRef Loops,
InsertPointTy ComputeIP);
 
+  /// Get the default alignment value for given target
+  ///
+  /// \param TargetTriple   Target triple
+  /// \param Features   StringMap which describes extra CPU features
+  static unsigned getOpenMPDefaultSimdAlign(const Triple &TargetTriple,
+const StringMap &Features);
+
 private:
   /// Modifies the canonical loop to be a statically-scheduled workshare loop.
   ///
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -500,8 +500,6 @@
   auto target_info = TargetInfo::CreateTargetInfo(
   m_compiler->getDiagnostics(), m_compiler->getInvocation().TargetOpts);
   if (log) {
-LLDB_LOGF(log, "Using SIMD alignment: %d",
-  target_info->getSimdDefaultAlign());
 LLDB_LOGF(log, "Target datalayout string: '%s'",
   target_info->getDataLayoutString());
 LLDB_LOGF(log, "Target ABI: '%s'", target_info->getABI().str().c_str());
Index: clang/lib/Basic/Targets/X86.cpp
===
--- clang/lib/Basic/Targets/X86.cpp
+++ clang/lib/Basic/Targets/X86.cpp
@@ -400,9 +400,6 @@
 return false;
   }
 
-  SimdDefaultAlign =
-  hasFeature("avx512f") ? 512 : hasFeature("avx") ? 256 : 128;
-
   // FIXME: We should allow long double type on 32-bits to match with GCC.
   // This requires backend to be able to lower f80 without x87 first.
   if (!HasX87 && LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -49,7 +49,6 @@
 SuitableAlign = 128;
 LargeArrayMinWidth = 128;
 LargeArrayAlign = 128;
-SimdDefaultAlign = 128;
 SigAtomicType = SignedLong;
 LongDoubleWidth = LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::IEEEquad();
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -87,7 +87,6 @@
   PPCTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple) {
 SuitableAlign = 128;
-SimdDefaultAlign = 128;
 LongDoubleWidth = LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::PPCDoubleDouble();
 HasStrictFP = true;
Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -119,7 +119,6 @@
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 0;
   MaxVectorAlign = 0;
   MaxTLSAlign = 0;
-  SimdDefaultAlign = 0;
   SizeType = UnsignedLong;
   PtrDiffType = SignedLong;
   IntMaxType = SignedLongLong;
Index: clang/lib/AST/CMakeLists.txt
==

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Dominik Adamski via Phabricator via lldb-commits]

domada updated this revision to Diff 495609.
domada added a reviewer: akyrtzi.
domada added a comment.

Added changes in `clang/lib/AST/CMakeLists.txt` to address build issue reported 
by @akyrtzi .

I modified CMakeLists.txt so that it requires generation of missing 
`Attributes.inc`.

@akyrtzi  Please let me know if it solves your issue


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

https://reviews.llvm.org/D141910

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/CMakeLists.txt
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -3053,6 +3053,23 @@
   Builder.CreateBr(NewBlocks.front());
 }
 
+unsigned
+OpenMPIRBuilder::getOpenMPDefaultSimdAlign(const Triple &TargetTriple,
+   const StringMap &Features) {
+  if (TargetTriple.isX86()) {
+if (Features.lookup("avx512f"))
+  return 512;
+else if (Features.lookup("avx"))
+  return 256;
+return 128;
+  }
+  if (TargetTriple.isPPC())
+return 128;
+  if (TargetTriple.isWasm())
+return 128;
+  return 0;
+}
+
 void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop,
 MapVector AlignedVars,
 Value *IfCond, OrderKind Order,
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -502,6 +502,13 @@
ArrayRef Loops,
InsertPointTy ComputeIP);
 
+  /// Get the default alignment value for given target
+  ///
+  /// \param TargetTriple   Target triple
+  /// \param Features   StringMap which describes extra CPU features
+  static unsigned getOpenMPDefaultSimdAlign(const Triple &TargetTriple,
+const StringMap &Features);
+
 private:
   /// Modifies the canonical loop to be a statically-scheduled workshare loop.
   ///
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -500,8 +500,6 @@
   auto target_info = TargetInfo::CreateTargetInfo(
   m_compiler->getDiagnostics(), m_compiler->getInvocation().TargetOpts);
   if (log) {
-LLDB_LOGF(log, "Using SIMD alignment: %d",
-  target_info->getSimdDefaultAlign());
 LLDB_LOGF(log, "Target datalayout string: '%s'",
   target_info->getDataLayoutString());
 LLDB_LOGF(log, "Target ABI: '%s'", target_info->getABI().str().c_str());
Index: clang/lib/Basic/Targets/X86.cpp
===
--- clang/lib/Basic/Targets/X86.cpp
+++ clang/lib/Basic/Targets/X86.cpp
@@ -400,9 +400,6 @@
 return false;
   }
 
-  SimdDefaultAlign =
-  hasFeature("avx512f") ? 512 : hasFeature("avx") ? 256 : 128;
-
   // FIXME: We should allow long double type on 32-bits to match with GCC.
   // This requires backend to be able to lower f80 without x87 first.
   if (!HasX87 && LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -49,7 +49,6 @@
 SuitableAlign = 128;
 LargeArrayMinWidth = 128;
 LargeArrayAlign = 128;
-SimdDefaultAlign = 128;
 SigAtomicType = SignedLong;
 LongDoubleWidth = LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::IEEEquad();
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -87,7 +87,6 @@
   PPCTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple) {
 SuitableAlign = 128;
-SimdDefaultAlign = 128;
 LongDoubleWidth = LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::PPCDoubleDouble();
 HasStrictFP = true;
Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -119,7 +119,6 @@
   MaxAtomicPromoteWidth = MaxAtomicInlineW

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Argyrios Kyrtzidis via Phabricator via lldb-commits]

akyrtzi added a comment.

I've reverted this change from `main` branch, let me know if there's anything I 
can do to help with addressing the build issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141910

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Argyrios Kyrtzidis via Phabricator via lldb-commits]

akyrtzi added a comment.

Hi @domada, these changes break compilation of clang, with such build error:

  FAILED: tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/ASTContext.cpp.o 
  
  In file included from /llvm-project/clang/lib/AST/ASTContext.cpp:81:
  In file included from 
/llvm-project/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:17:
  In file included from 
/llvm-project/llvm/include/llvm/Analysis/MemorySSAUpdater.h:37:
  In file included from /llvm-project/llvm/include/llvm/Analysis/MemorySSA.h:93:
  In file included from 
/llvm-project/llvm/include/llvm/Analysis/AliasAnalysis.h:44:
  In file included from /llvm-project/llvm/include/llvm/IR/PassManager.h:45:
  In file included from /llvm-project/llvm/include/llvm/IR/Function.h:25:
  In file included from /llvm-project/llvm/include/llvm/IR/Argument.h:17:
  /llvm-project/llvm/include/llvm/IR/Attributes.h:90:14: fatal error: 
'llvm/IR/Attributes.inc' file not found

Reproduction steps:

1. Configure to build clang using `ninja`:

  cmake -G Ninja /path/to/llvm-project/llvm \
-DLLVM_TARGETS_TO_BUILD="X86;ARM;AArch64" \
-DCMAKE_BUILD_TYPE:STRING=Release \
-DLLVM_ENABLE_PROJECTS="clang"

2. Build `ASTContext.cpp.o` with a clean build directory.

  ninja clean && ninja 
tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/ASTContext.cpp.o

Are you able to take a look? 🙏


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141910

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Dominik Adamski via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGca446037af01: [OpenMP][OMPIRBuilder]Move SIMD alignment 
calculation to LLVM Frontend (authored by domada).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141910

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -3053,6 +3053,23 @@
   Builder.CreateBr(NewBlocks.front());
 }
 
+unsigned
+OpenMPIRBuilder::getOpenMPDefaultSimdAlign(const Triple &TargetTriple,
+   const StringMap &Features) {
+  if (TargetTriple.isX86()) {
+if (Features.lookup("avx512f"))
+  return 512;
+else if (Features.lookup("avx"))
+  return 256;
+return 128;
+  }
+  if (TargetTriple.isPPC())
+return 128;
+  if (TargetTriple.isWasm())
+return 128;
+  return 0;
+}
+
 void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop,
 MapVector AlignedVars,
 Value *IfCond, OrderKind Order,
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -502,6 +502,13 @@
ArrayRef Loops,
InsertPointTy ComputeIP);
 
+  /// Get the default alignment value for given target
+  ///
+  /// \param TargetTriple   Target triple
+  /// \param Features   StringMap which describes extra CPU features
+  static unsigned getOpenMPDefaultSimdAlign(const Triple &TargetTriple,
+const StringMap &Features);
+
 private:
   /// Modifies the canonical loop to be a statically-scheduled workshare loop.
   ///
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -500,8 +500,6 @@
   auto target_info = TargetInfo::CreateTargetInfo(
   m_compiler->getDiagnostics(), m_compiler->getInvocation().TargetOpts);
   if (log) {
-LLDB_LOGF(log, "Using SIMD alignment: %d",
-  target_info->getSimdDefaultAlign());
 LLDB_LOGF(log, "Target datalayout string: '%s'",
   target_info->getDataLayoutString());
 LLDB_LOGF(log, "Target ABI: '%s'", target_info->getABI().str().c_str());
Index: clang/lib/Basic/Targets/X86.cpp
===
--- clang/lib/Basic/Targets/X86.cpp
+++ clang/lib/Basic/Targets/X86.cpp
@@ -400,9 +400,6 @@
 return false;
   }
 
-  SimdDefaultAlign =
-  hasFeature("avx512f") ? 512 : hasFeature("avx") ? 256 : 128;
-
   // FIXME: We should allow long double type on 32-bits to match with GCC.
   // This requires backend to be able to lower f80 without x87 first.
   if (!HasX87 && LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -49,7 +49,6 @@
 SuitableAlign = 128;
 LargeArrayMinWidth = 128;
 LargeArrayAlign = 128;
-SimdDefaultAlign = 128;
 SigAtomicType = SignedLong;
 LongDoubleWidth = LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::IEEEquad();
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -87,7 +87,6 @@
   PPCTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple) {
 SuitableAlign = 128;
-SimdDefaultAlign = 128;
 LongDoubleWidth = LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::PPCDoubleDouble();
 HasStrictFP = true;
Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -119,7 +119,6 @@
   MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 0;
   MaxVectorAlign = 0;
   MaxTLSAlign = 0;
-  SimdDefaultAlign = 0;

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Johannes Doerfert via Phabricator via lldb-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141910

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Jan Sjödin via Phabricator via lldb-commits]

jsjodin accepted this revision.
jsjodin added a comment.
This revision is now accepted and ready to land.

This looks good to me. Have @jdoerfert approve as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141910

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D141910: [OpenMP][OMPIRBuilder]Move SIMD alignment calculation to LLVM Frontend

[Dominik Adamski via Phabricator via lldb-commits]

domada created this revision.
domada added reviewers: dpalermo, skatrak, kiranktp, RogerV-AMD, NimishMishra, 
jsjodin.
Herald added subscribers: pmatos, asb, guansong, kbarton, hiraditya, 
jgravelle-google, sbc100, yaxunl, nemanjai, dschuff.
Herald added a project: All.
domada requested review of this revision.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, sstefan1, 
aheejin.
Herald added a reviewer: jdoerfert.
Herald added projects: clang, LLDB, LLVM.

Currently default simd alignment is defined by Clang specific TargetInfo class. 
This class cannot be reused for LLVM Flang. That's why default simd alignment 
calculation has been moved to OMPIRBuilder which is common for Flang and Clang.

Previous attempt: https://reviews.llvm.org/D138496 was wrong because the 
default alignment depended on the number of built LLVM targets.

If we wanted to calculate the default alignment for PPC and we hadn't specified 
PPC LLVM target to build, then we would get 0 as the alignment because 
OMPIRBuilder couldn't create PPCTargetMachine object and it returned 0 as the 
default value. If PPC LLVM target had been built earlier, then OMPIRBuilder 
could have created PPCTargetMachine object and it would have returned 128.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141910

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -3039,6 +3039,23 @@
   Builder.CreateBr(NewBlocks.front());
 }
 
+unsigned
+OpenMPIRBuilder::getOpenMPDefaultSimdAlign(const Triple &TargetTriple,
+   const StringMap &Features) {
+  if (TargetTriple.isX86()) {
+if (Features.lookup("avx512f"))
+  return 512;
+else if (Features.lookup("avx"))
+  return 256;
+return 128;
+  }
+  if (TargetTriple.isPPC())
+return 128;
+  if (TargetTriple.isWasm())
+return 128;
+  return 0;
+}
+
 void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop,
 MapVector AlignedVars,
 Value *IfCond, OrderKind Order,
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -502,6 +502,13 @@
ArrayRef Loops,
InsertPointTy ComputeIP);
 
+  /// Get the default alignment value for given target
+  ///
+  /// \param TargetTriple   Target triple
+  /// \param Features   StringMap which describes extra CPU features
+  static unsigned getOpenMPDefaultSimdAlign(const Triple &TargetTriple,
+const StringMap &Features);
+
 private:
   /// Modifies the canonical loop to be a statically-scheduled workshare loop.
   ///
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -500,8 +500,6 @@
   auto target_info = TargetInfo::CreateTargetInfo(
   m_compiler->getDiagnostics(), m_compiler->getInvocation().TargetOpts);
   if (log) {
-LLDB_LOGF(log, "Using SIMD alignment: %d",
-  target_info->getSimdDefaultAlign());
 LLDB_LOGF(log, "Target datalayout string: '%s'",
   target_info->getDataLayoutString());
 LLDB_LOGF(log, "Target ABI: '%s'", target_info->getABI().str().c_str());
Index: clang/lib/Basic/Targets/X86.cpp
===
--- clang/lib/Basic/Targets/X86.cpp
+++ clang/lib/Basic/Targets/X86.cpp
@@ -400,9 +400,6 @@
 return false;
   }
 
-  SimdDefaultAlign =
-  hasFeature("avx512f") ? 512 : hasFeature("avx") ? 256 : 128;
-
   // FIXME: We should allow long double type on 32-bits to match with GCC.
   // This requires backend to be able to lower f80 without x87 first.
   if (!HasX87 && LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -49,7 +49,6 @@
 SuitableAlign = 128;
 LargeArrayMinWidth = 128;
 LargeArrayAlign = 128;
-SimdDefaultAlign = 128;