[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

Thanks for the update @asb. LGTM.


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

https://reviews.llvm.org/D142327

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

[PATCH] D156438: [Docs] Fix code-blocks missing colon

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG31747668: [Docs] Fix code-blocks missing a colon and a 
newline (authored by rogfer01).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156438

Files:
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/bad-signal-to-kill-thread.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/spuriously-wake-up-functions.rst
  
clang-tools-extra/docs/clang-tidy/checks/concurrency/thread-canceltype-asynchronous.rst
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticGroups.td
  llvm/docs/ORCv2.rst

Index: llvm/docs/ORCv2.rst
===
--- llvm/docs/ORCv2.rst
+++ llvm/docs/ORCv2.rst
@@ -311,7 +311,7 @@
 further materialization, for example: "foo" = 0x1234. One use case for
 absolute symbols is allowing resolution of process symbols. E.g.
 
-.. code-block: c++
+.. code-block:: c++
 
   JD.define(absoluteSymbols(SymbolMap({
   { Mangle("printf"),
@@ -334,7 +334,7 @@
 some calls. We could bake the address of your object into the library, but then
 it would need to be recompiled for each session:
 
-.. code-block: c++
+.. code-block:: c++
 
   // From standard library for JIT'd code:
 
@@ -347,7 +347,7 @@
 
 We can turn this into a symbolic reference in the JIT standard library:
 
-.. code-block: c++
+.. code-block:: c++
 
   extern MyJIT *__MyJITInstance;
 
@@ -356,7 +356,7 @@
 And then make our JIT object visible to the JIT standard library with an
 absolute symbol definition when the JIT is started:
 
-.. code-block: c++
+.. code-block:: c++
 
   MyJIT J = ...;
 
@@ -379,7 +379,7 @@
 used when the ``log`` symbol is referenced by setting up an alias at JIT startup
 time:
 
-.. code-block: c++
+.. code-block:: c++
 
   auto  = ... ;
 
@@ -397,7 +397,7 @@
 JITDylib. The ``reexports`` function provides the same functionality, but
 operates across JITDylib boundaries. E.g.
 
-.. code-block: c++
+.. code-block:: c++
 
   auto  = ... ;
   auto  = ... ;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1299,10 +1299,12 @@
 Results can be filtered by function name by passing
 `-mllvm -filter-print-funcs=foo`, where `foo` is the target function's name.
 
-   .. code-block: console
+   .. code-block:: console
+
   clang -c a.cpp -Rpass-analysis=stack-frame-layout -mllvm -filter-print-funcs=foo
 
-   .. code-block: console
+   .. code-block:: console
+
   clang -c a.cpp -Rpass-analysis=stack-frame-layout -foptimization-record-file=
 }];
 }
@@ -1394,19 +1396,22 @@
 1. As a limit at a specific point in a file, using the ``clang max_tokens_here``
pragma:
 
-   .. code-block: c++
+   .. code-block:: c++
+
   #pragma clang max_tokens_here 1234
 
 2. As a per-translation unit limit, using the ``-fmax-tokens=`` command-line
flag:
 
-   .. code-block: console
+   .. code-block:: console
+
   clang -c a.cpp -fmax-tokens=1234
 
 3. As a per-translation unit limit using the ``clang max_tokens_total`` pragma,
which works like and overrides the ``-fmax-tokens=`` flag:
 
-   .. code-block: c++
+   .. code-block:: c++
+
   #pragma clang max_tokens_total 1234
 
 These limits can be helpful in limiting code growth through included files.
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -1858,7 +1858,8 @@
 variable, a function or method, a function parameter, an enumeration, an
 enumerator, a non-static data member, or a label.
 
-.. code-block: c++
+.. code-block:: c++
+
   #include 
 
   [[maybe_unused]] void f([[maybe_unused]] bool thing1,
@@ -1887,7 +1888,8 @@
 differing string literals, it is unspecified which one will be used by Clang
 in any resulting diagnostics.
 
-.. code-block: c++
+.. code-block:: c++
+
   struct [[nodiscard]] error_info { /*...*/ };
   error_info enable_missile_safety_mode();
 
@@ -1904,7 +1906,8 @@
 ``[[nodiscard]]`` will also diagnose. This also applies to type conversions that
 use the annotated ``[[nodiscard]]`` constructor or result in an annotated type.
 
-.. code-block: c++
+.. code-block:: c++
+
   struct [[nodiscard]] marked_type {/*..*/ };
   struct marked_ctor {
 [[nodiscard]] marked_ctor();
@@ -5685,12 +5688,12 @@
 ``noderef`` is currently only supported for pointers and arrays and not usable
 for references or Objective-C object pointers.
 
-.. code-block: c++
+.. code-block:: c++
 
   int x = 2;
   int __attribute__((noderef))  = x;  // warning: 'noderef' can only be used on an array or pointer type
 
-.. code-block: objc
+.. 

[PATCH] D156438: [Docs] Fix code-blocks missing colon

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 created this revision.
Herald added a reviewer: aaron.ballman.
Herald added subscribers: PiotrZSL, carlosgalvezp.
Herald added a reviewer: njames93.
Herald added a project: All.
rogfer01 requested review of this revision.
Herald added projects: clang, LLVM, clang-tools-extra.
Herald added subscribers: cfe-commits, llvm-commits.

I was reading ORCv2 HTML documentation and I found the text mentioned examples 
that were not present. Turns out there were some syntax issues in the 
reStructuedText file that prevented rendering them.

I did a grep through the rest of the docs and I found a few other instances of 
the problem, including missing newlines.

I was pleased to discover that clang attributes  `nodiscard` / 
`warn_unused_result` had a couple useful example that had never been rendered!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156438

Files:
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/bad-signal-to-kill-thread.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone/spuriously-wake-up-functions.rst
  
clang-tools-extra/docs/clang-tidy/checks/concurrency/thread-canceltype-asynchronous.rst
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticGroups.td
  llvm/docs/ORCv2.rst

Index: llvm/docs/ORCv2.rst
===
--- llvm/docs/ORCv2.rst
+++ llvm/docs/ORCv2.rst
@@ -311,7 +311,7 @@
 further materialization, for example: "foo" = 0x1234. One use case for
 absolute symbols is allowing resolution of process symbols. E.g.
 
-.. code-block: c++
+.. code-block:: c++
 
   JD.define(absoluteSymbols(SymbolMap({
   { Mangle("printf"),
@@ -334,7 +334,7 @@
 some calls. We could bake the address of your object into the library, but then
 it would need to be recompiled for each session:
 
-.. code-block: c++
+.. code-block:: c++
 
   // From standard library for JIT'd code:
 
@@ -347,7 +347,7 @@
 
 We can turn this into a symbolic reference in the JIT standard library:
 
-.. code-block: c++
+.. code-block:: c++
 
   extern MyJIT *__MyJITInstance;
 
@@ -356,7 +356,7 @@
 And then make our JIT object visible to the JIT standard library with an
 absolute symbol definition when the JIT is started:
 
-.. code-block: c++
+.. code-block:: c++
 
   MyJIT J = ...;
 
@@ -379,7 +379,7 @@
 used when the ``log`` symbol is referenced by setting up an alias at JIT startup
 time:
 
-.. code-block: c++
+.. code-block:: c++
 
   auto  = ... ;
 
@@ -397,7 +397,7 @@
 JITDylib. The ``reexports`` function provides the same functionality, but
 operates across JITDylib boundaries. E.g.
 
-.. code-block: c++
+.. code-block:: c++
 
   auto  = ... ;
   auto  = ... ;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1299,10 +1299,12 @@
 Results can be filtered by function name by passing
 `-mllvm -filter-print-funcs=foo`, where `foo` is the target function's name.
 
-   .. code-block: console
+   .. code-block:: console
+
   clang -c a.cpp -Rpass-analysis=stack-frame-layout -mllvm -filter-print-funcs=foo
 
-   .. code-block: console
+   .. code-block:: console
+
   clang -c a.cpp -Rpass-analysis=stack-frame-layout -foptimization-record-file=
 }];
 }
@@ -1394,19 +1396,22 @@
 1. As a limit at a specific point in a file, using the ``clang max_tokens_here``
pragma:
 
-   .. code-block: c++
+   .. code-block:: c++
+
   #pragma clang max_tokens_here 1234
 
 2. As a per-translation unit limit, using the ``-fmax-tokens=`` command-line
flag:
 
-   .. code-block: console
+   .. code-block:: console
+
   clang -c a.cpp -fmax-tokens=1234
 
 3. As a per-translation unit limit using the ``clang max_tokens_total`` pragma,
which works like and overrides the ``-fmax-tokens=`` flag:
 
-   .. code-block: c++
+   .. code-block:: c++
+
   #pragma clang max_tokens_total 1234
 
 These limits can be helpful in limiting code growth through included files.
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -1858,7 +1858,8 @@
 variable, a function or method, a function parameter, an enumeration, an
 enumerator, a non-static data member, or a label.
 
-.. code-block: c++
+.. code-block:: c++
+
   #include 
 
   [[maybe_unused]] void f([[maybe_unused]] bool thing1,
@@ -1887,7 +1888,8 @@
 differing string literals, it is unspecified which one will be used by Clang
 in any resulting diagnostics.
 
-.. code-block: c++
+.. code-block:: c++
+
   struct [[nodiscard]] error_info { /*...*/ };
   error_info enable_missile_safety_mode();
 
@@ -1904,7 +1906,8 @@
 ``[[nodiscard]]`` will also diagnose. This also applies to type conversions that
 use the annotated ``[[nodiscard]]`` constructor or result in an 

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

I didn't have libcxx handy but @SixWeining testcase also fails with libstdcxx 
so I expanded what C++ does and removed the templates.

With the new patch this does not fail anymore.

  typedef decltype((int *)2 - (int *)1) my_ptrdiff_t;
  
  struct my_lambda {
bool operator()(int i) { return i == 2; }
  };
  
  struct my_iter_pred {
my_lambda _M_pred;
  
explicit my_iter_pred(my_lambda __pred)
: _M_pred(/* move */ static_cast(__pred)) {}
  
bool operator()(int *__it) { return bool(_M_pred(*__it)); }
  };
  
  my_ptrdiff_t __my_count_if(int *__first, int *__last, my_iter_pred __pred) {
my_ptrdiff_t __n = 0;
for (; __first != __last; ++__first)
  if (__pred(__first))
++__n;
return __n;
  }
  
  inline my_iter_pred my_pred_iter(my_lambda __my_lambda) {
return my_iter_pred(/* move */ static_cast(__my_lambda));
  }
  
  inline my_ptrdiff_t my_count_if(int *__first, int *__last,
  my_lambda __my_lambda) {
return __my_count_if(__first, __last, my_pred_iter(__my_lambda));
  }
  
  int main() {
int v[] = {1, 2, 3, 2, 2};
return my_count_if(v, v + sizeof(v) / sizeof(*v), my_lambda());
  }

I tried to remove the `my_pred_iter` / `my_iter_pred` wrapper but then the 
original patch would not fail anymore.

Hope this helps.


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

https://reviews.llvm.org/D142327

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

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

I didn't have libcxx handy but @SixWeining testcase also failed with libstdcxx 
so I made a slightly smaller standalone testcase based on what libstdcxx does 
and I manually replaced all the template stuff.

  typedef decltype((int *)2 - (int *)1) my_ptrdiff_t;
  
  struct my_lambda {
bool operator()(int i) { return i == 2; }
  };
  
  struct my_iter_pred {
my_lambda _M_pred;
  
bool operator()(int *__it) { return bool(_M_pred(*__it)); }
  };
  
  my_ptrdiff_t __my_count_if(int *__first, int *__last, my_iter_pred __pred) {
my_ptrdiff_t __n = 0;
for (; __first != __last; ++__first)
  if (__pred(__first))
++__n;
return __n;
  }
  
  inline my_iter_pred my_pred_iter(my_lambda __my_lambda) {
return my_iter_pred(/* move */ static_cast(_my_lambda));
  }
  
  inline my_ptrdiff_t my_count_if(int *__first, int *__last,
  my_lambda __my_lambda) {
return __my_count_if(__first, __last, my_pred_iter(__my_lambda));
  }
  
  int main() {
int v[] = {1, 2, 3, 2, 2};
return my_count_if(v, v + sizeof(v) / sizeof(*v), my_lambda());
  }

I tried to simplify it further by removing the `my_pred_iter` / `my_iter_pred` 
adapter but then it would not crash anymore. Hope this helps.


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

https://reviews.llvm.org/D142327

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

[PATCH] D142327: [clang][RISCV] Fix ABI handling of empty structs with hard FP calling conventions in C++

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added inline comments.



Comment at: clang/lib/CodeGen/Targets/RISCV.cpp:178
   return false;
-if (isEmptyRecord(getContext(), Ty, true))
+if (isEmptyRecord(getContext(), Ty, true, true))
   return true;

I've observed (based on manually added tracing) that we may return true here 
with `Field1Ty == nullptr` and `Field2Ty == nullptr`.

Then `RISCVABIInfo::coerceAndExpandFPCCEligibleStruct` receives these two types 
and appends `Field1Ty` it into `UnpaddedCoerceElts` and then if `Field2Ty == 
nullptr` it calls `ABIArgInfo::getCoerceAndExpand` (around line 280 of 
`clang/lib/CodeGen/Targets/RISCV.cpp`) which then tries to do a `dyn_cast` on a 
null value and then an assertion fires (around line 256 of 
`clang/include/clang/CodeGen/CGFunctionInfo.h`)

I can reproduce this with the llvm-testsuite. Apologies that I have not managed 
to create a small reproducer yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142327

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

[PATCH] D153111: [clang][Serialization][RISCV] Increase the number of reserved predefined type IDs

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfa45f81ff7ea: [clang][Serialization][RISCV] Increase the 
number of reserved predefined type… (authored by rogfer01).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153111

Files:
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/Serialization/ASTReader.cpp
  clang/test/Modules/embed-files-compressed.cpp
  clang/test/Modules/empty.modulemap


Index: clang/test/Modules/empty.modulemap
===
--- clang/test/Modules/empty.modulemap
+++ clang/test/Modules/empty.modulemap
@@ -13,8 +13,8 @@
 // The module file should be identical each time we produce it.
 // RUN: diff %t/base.pcm %t/check.pcm
 //
-// We expect an empty module to be less than 40KB (and at least 10K, for now).
+// We expect an empty module to be less than 60KB (and at least 10K, for now).
 // RUN: wc -c %t/base.pcm | FileCheck --check-prefix=CHECK-SIZE %s
-// CHECK-SIZE: {{(^|[^0-9])[123][0-9][0-9][0-9][0-9]($|[^0-9])}}
+// CHECK-SIZE: {{(^|[^0-9])[1-5][0-9][0-9][0-9][0-9]($|[^0-9])}}
 
 module empty { header "Inputs/empty.h" export * }
Index: clang/test/Modules/embed-files-compressed.cpp
===
--- clang/test/Modules/embed-files-compressed.cpp
+++ clang/test/Modules/embed-files-compressed.cpp
@@ -17,7 +17,7 @@
 // RUN: %clang_cc1 -fmodules -I%t -fmodules-cache-path=%t -fmodule-name=a 
-emit-module %t/modulemap -fmodules-embed-all-files -o %t/a.pcm
 //
 // The above embeds ~4.5MB of highly-predictable /s and \ns into the pcm file.
-// Check that the resulting file is under 40KB:
+// Check that the resulting file is under 60KB:
 //
 // RUN: wc -c %t/a.pcm | FileCheck --check-prefix=CHECK-SIZE %s
-// CHECK-SIZE: {{(^|[^0-9])[123][0-9][0-9][0-9][0-9]($|[^0-9])}}
+// CHECK-SIZE: {{(^|[^0-9])[1-5][0-9][0-9][0-9][0-9]($|[^0-9])}}
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -6983,6 +6983,10 @@
   if (Index < NUM_PREDEF_TYPE_IDS) {
 QualType T;
 switch ((PredefinedTypeIDs)Index) {
+case PREDEF_TYPE_LAST_ID:
+  // We should never use this one.
+  llvm_unreachable("Invalid predefined type");
+  break;
 case PREDEF_TYPE_NULL_ID:
   return QualType();
 case PREDEF_TYPE_VOID_ID:
Index: clang/include/clang/Serialization/ASTBitCodes.h
===
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -1099,6 +1099,8 @@
 // \brief WebAssembly reference types with auto numeration
 #define WASM_TYPE(Name, Id, SingletonId) PREDEF_TYPE_##Id##_ID,
 #include "clang/Basic/WebAssemblyReferenceTypes.def"
+  // Sentinel value. Considered a predefined type but not useable as one.
+  PREDEF_TYPE_LAST_ID
 };
 
 /// The number of predefined type IDs that are reserved for
@@ -1106,7 +1108,13 @@
 ///
 /// Type IDs for non-predefined types will start at
 /// NUM_PREDEF_TYPE_IDs.
-const unsigned NUM_PREDEF_TYPE_IDS = 300;
+const unsigned NUM_PREDEF_TYPE_IDS = 500;
+
+// Ensure we do not overrun the predefined types we reserved
+// in the enum PredefinedTypeIDs above.
+static_assert(PREDEF_TYPE_LAST_ID < NUM_PREDEF_TYPE_IDS,
+  "Too many enumerators in PredefinedTypeIDs. Review the value of "
+  "NUM_PREDEF_TYPE_IDS");
 
 /// Record codes for each kind of type.
 ///


Index: clang/test/Modules/empty.modulemap
===
--- clang/test/Modules/empty.modulemap
+++ clang/test/Modules/empty.modulemap
@@ -13,8 +13,8 @@
 // The module file should be identical each time we produce it.
 // RUN: diff %t/base.pcm %t/check.pcm
 //
-// We expect an empty module to be less than 40KB (and at least 10K, for now).
+// We expect an empty module to be less than 60KB (and at least 10K, for now).
 // RUN: wc -c %t/base.pcm | FileCheck --check-prefix=CHECK-SIZE %s
-// CHECK-SIZE: {{(^|[^0-9])[123][0-9][0-9][0-9][0-9]($|[^0-9])}}
+// CHECK-SIZE: {{(^|[^0-9])[1-5][0-9][0-9][0-9][0-9]($|[^0-9])}}
 
 module empty { header "Inputs/empty.h" export * }
Index: clang/test/Modules/embed-files-compressed.cpp
===
--- clang/test/Modules/embed-files-compressed.cpp
+++ clang/test/Modules/embed-files-compressed.cpp
@@ -17,7 +17,7 @@
 // RUN: %clang_cc1 -fmodules -I%t -fmodules-cache-path=%t -fmodule-name=a -emit-module %t/modulemap -fmodules-embed-all-files -o %t/a.pcm
 //
 // The above embeds ~4.5MB of highly-predictable /s and \ns into the pcm file.
-// Check that the resulting file is under 40KB:
+// Check that the 

[PATCH] D153111: [clang][Serialization][RISCV] Increase the number of reserved predefined type IDs

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 updated this revision to Diff 532546.
rogfer01 added a comment.

ChangeLog:

- Update clang tests that now, on RISC-V only, observe a larger precompiled 
module file


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

https://reviews.llvm.org/D153111

Files:
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/Serialization/ASTReader.cpp
  clang/test/Modules/embed-files-compressed.cpp
  clang/test/Modules/empty.modulemap


Index: clang/test/Modules/empty.modulemap
===
--- clang/test/Modules/empty.modulemap
+++ clang/test/Modules/empty.modulemap
@@ -13,8 +13,8 @@
 // The module file should be identical each time we produce it.
 // RUN: diff %t/base.pcm %t/check.pcm
 //
-// We expect an empty module to be less than 40KB (and at least 10K, for now).
+// We expect an empty module to be less than 60KB (and at least 10K, for now).
 // RUN: wc -c %t/base.pcm | FileCheck --check-prefix=CHECK-SIZE %s
-// CHECK-SIZE: {{(^|[^0-9])[123][0-9][0-9][0-9][0-9]($|[^0-9])}}
+// CHECK-SIZE: {{(^|[^0-9])[1-5][0-9][0-9][0-9][0-9]($|[^0-9])}}
 
 module empty { header "Inputs/empty.h" export * }
Index: clang/test/Modules/embed-files-compressed.cpp
===
--- clang/test/Modules/embed-files-compressed.cpp
+++ clang/test/Modules/embed-files-compressed.cpp
@@ -17,7 +17,7 @@
 // RUN: %clang_cc1 -fmodules -I%t -fmodules-cache-path=%t -fmodule-name=a 
-emit-module %t/modulemap -fmodules-embed-all-files -o %t/a.pcm
 //
 // The above embeds ~4.5MB of highly-predictable /s and \ns into the pcm file.
-// Check that the resulting file is under 40KB:
+// Check that the resulting file is under 60KB:
 //
 // RUN: wc -c %t/a.pcm | FileCheck --check-prefix=CHECK-SIZE %s
-// CHECK-SIZE: {{(^|[^0-9])[123][0-9][0-9][0-9][0-9]($|[^0-9])}}
+// CHECK-SIZE: {{(^|[^0-9])[1-5][0-9][0-9][0-9][0-9]($|[^0-9])}}
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -6983,6 +6983,10 @@
   if (Index < NUM_PREDEF_TYPE_IDS) {
 QualType T;
 switch ((PredefinedTypeIDs)Index) {
+case PREDEF_TYPE_LAST_ID:
+  // We should never use this one.
+  llvm_unreachable("Invalid predefined type");
+  break;
 case PREDEF_TYPE_NULL_ID:
   return QualType();
 case PREDEF_TYPE_VOID_ID:
Index: clang/include/clang/Serialization/ASTBitCodes.h
===
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -1099,6 +1099,8 @@
 // \brief WebAssembly reference types with auto numeration
 #define WASM_TYPE(Name, Id, SingletonId) PREDEF_TYPE_##Id##_ID,
 #include "clang/Basic/WebAssemblyReferenceTypes.def"
+  // Sentinel value. Considered a predefined type but not useable as one.
+  PREDEF_TYPE_LAST_ID
 };
 
 /// The number of predefined type IDs that are reserved for
@@ -1106,7 +1108,13 @@
 ///
 /// Type IDs for non-predefined types will start at
 /// NUM_PREDEF_TYPE_IDs.
-const unsigned NUM_PREDEF_TYPE_IDS = 300;
+const unsigned NUM_PREDEF_TYPE_IDS = 500;
+
+// Ensure we do not overrun the predefined types we reserved
+// in the enum PredefinedTypeIDs above.
+static_assert(PREDEF_TYPE_LAST_ID < NUM_PREDEF_TYPE_IDS,
+  "Too many enumerators in PredefinedTypeIDs. Review the value of "
+  "NUM_PREDEF_TYPE_IDS");
 
 /// Record codes for each kind of type.
 ///


Index: clang/test/Modules/empty.modulemap
===
--- clang/test/Modules/empty.modulemap
+++ clang/test/Modules/empty.modulemap
@@ -13,8 +13,8 @@
 // The module file should be identical each time we produce it.
 // RUN: diff %t/base.pcm %t/check.pcm
 //
-// We expect an empty module to be less than 40KB (and at least 10K, for now).
+// We expect an empty module to be less than 60KB (and at least 10K, for now).
 // RUN: wc -c %t/base.pcm | FileCheck --check-prefix=CHECK-SIZE %s
-// CHECK-SIZE: {{(^|[^0-9])[123][0-9][0-9][0-9][0-9]($|[^0-9])}}
+// CHECK-SIZE: {{(^|[^0-9])[1-5][0-9][0-9][0-9][0-9]($|[^0-9])}}
 
 module empty { header "Inputs/empty.h" export * }
Index: clang/test/Modules/embed-files-compressed.cpp
===
--- clang/test/Modules/embed-files-compressed.cpp
+++ clang/test/Modules/embed-files-compressed.cpp
@@ -17,7 +17,7 @@
 // RUN: %clang_cc1 -fmodules -I%t -fmodules-cache-path=%t -fmodule-name=a -emit-module %t/modulemap -fmodules-embed-all-files -o %t/a.pcm
 //
 // The above embeds ~4.5MB of highly-predictable /s and \ns into the pcm file.
-// Check that the resulting file is under 40KB:
+// Check that the resulting file is under 60KB:
 //
 // RUN: wc -c %t/a.pcm | FileCheck --check-prefix=CHECK-SIZE %s
-// CHECK-SIZE: 

[PATCH] D153111: [clang][Serialization][RISCV] Increase the number of reserved predefined type IDs

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Thanks @eopXD. Not sure why you didn't observe the failure. I understand your 
build has `-DLLVM_ENABLE_ASSERTIONS=ON`.

I investigated a bit the libcxx errors flagged by the precommit CI. I can get 
similar (but not exactly the same errors) if the build before this change is 
`-DLLVM_ENABLE_PROJECTS=clang -DLLVM_ENABLE_RUNTIMES=libcxx`, then I apply this 
change and then I run libcxx tests: several precompiled headers become stale 
after this change and would need to be rebuilt. Starting from a clean build 
directory avoids this issue.

I want to do a few more checks locally before committing. Sorry for the delay.


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

https://reviews.llvm.org/D153111

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

[PATCH] D153111: [clang][Serialization][RISCV] Increase the number of reserved predefined type IDs

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 updated this revision to Diff 532061.
rogfer01 added a comment.

ChangeLog:

- Add a sentinel and a `static_assert`


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

https://reviews.llvm.org/D153111

Files:
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/Serialization/ASTReader.cpp


Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -6983,6 +6983,10 @@
   if (Index < NUM_PREDEF_TYPE_IDS) {
 QualType T;
 switch ((PredefinedTypeIDs)Index) {
+case PREDEF_TYPE_LAST_ID:
+  // We should never use this one.
+  llvm_unreachable("Invalid predefined type");
+  break;
 case PREDEF_TYPE_NULL_ID:
   return QualType();
 case PREDEF_TYPE_VOID_ID:
Index: clang/include/clang/Serialization/ASTBitCodes.h
===
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -1099,6 +1099,8 @@
 // \brief WebAssembly reference types with auto numeration
 #define WASM_TYPE(Name, Id, SingletonId) PREDEF_TYPE_##Id##_ID,
 #include "clang/Basic/WebAssemblyReferenceTypes.def"
+  // Sentinel value. Considered a predefined type but not useable as one.
+  PREDEF_TYPE_LAST_ID
 };
 
 /// The number of predefined type IDs that are reserved for
@@ -1106,7 +1108,13 @@
 ///
 /// Type IDs for non-predefined types will start at
 /// NUM_PREDEF_TYPE_IDs.
-const unsigned NUM_PREDEF_TYPE_IDS = 300;
+const unsigned NUM_PREDEF_TYPE_IDS = 500;
+
+// Ensure we do not overrun the predefined types we reserved
+// in the enum PredefinedTypeIDs above.
+static_assert(PREDEF_TYPE_LAST_ID < NUM_PREDEF_TYPE_IDS,
+  "Too many enumerators in PredefinedTypeIDs. Review the value of "
+  "NUM_PREDEF_TYPE_IDS");
 
 /// Record codes for each kind of type.
 ///


Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -6983,6 +6983,10 @@
   if (Index < NUM_PREDEF_TYPE_IDS) {
 QualType T;
 switch ((PredefinedTypeIDs)Index) {
+case PREDEF_TYPE_LAST_ID:
+  // We should never use this one.
+  llvm_unreachable("Invalid predefined type");
+  break;
 case PREDEF_TYPE_NULL_ID:
   return QualType();
 case PREDEF_TYPE_VOID_ID:
Index: clang/include/clang/Serialization/ASTBitCodes.h
===
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -1099,6 +1099,8 @@
 // \brief WebAssembly reference types with auto numeration
 #define WASM_TYPE(Name, Id, SingletonId) PREDEF_TYPE_##Id##_ID,
 #include "clang/Basic/WebAssemblyReferenceTypes.def"
+  // Sentinel value. Considered a predefined type but not useable as one.
+  PREDEF_TYPE_LAST_ID
 };
 
 /// The number of predefined type IDs that are reserved for
@@ -1106,7 +1108,13 @@
 ///
 /// Type IDs for non-predefined types will start at
 /// NUM_PREDEF_TYPE_IDs.
-const unsigned NUM_PREDEF_TYPE_IDS = 300;
+const unsigned NUM_PREDEF_TYPE_IDS = 500;
+
+// Ensure we do not overrun the predefined types we reserved
+// in the enum PredefinedTypeIDs above.
+static_assert(PREDEF_TYPE_LAST_ID < NUM_PREDEF_TYPE_IDS,
+  "Too many enumerators in PredefinedTypeIDs. Review the value of "
+  "NUM_PREDEF_TYPE_IDS");
 
 /// Record codes for each kind of type.
 ///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153111: [clang][Serialization][RISCV] Increase the number of reserved predefined type IDs

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Let me check if I can add a `static_assert` to get a compile-time failure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153111

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

[PATCH] D152070: [2/11][Clang][RISCV] Expand all variants of RVV intrinsic tuple types

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

It looks like we need to increase `NUM_PREDEF_TYPE_IDS`. Let's continue the 
discussion on D153111 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152070

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

[PATCH] D153111: [clang][Serialization][RISCV] Increase the number of reserved predefined type IDs

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 created this revision.
rogfer01 added reviewers: eopXD, Eugene.Zelenko, craig.topper.
Herald added subscribers: jobnoorman, VincentWu, vkmr, luismarques, 
sameer.abuasal, s.egerton, Jim, benna, psnobl, PkmX, shiva0217, kito-cheng, 
simoncook, arichardson.
Herald added a project: All.
rogfer01 requested review of this revision.
Herald added subscribers: cfe-commits, wangpc.
Herald added a project: clang.

In https://reviews.llvm.org/D152070 we added many new intrinsic types required 
for the RISC-V Vector Extension.

This was crashing when loading the AST as those types are intrinsically added 
to the AST (they don't come from the disk).

  $ touch t.c
  $ clang -cc1 -triple riscv64 -w -emit-pch -o test.pch t.c
  $ clang -cc1 -triple riscv64 -w -x c -include-pch test.pch -ast-dump-all 
/dev/null

The total number required now by clang seems to go far beyond 300 so increasing 
the value to 500 seems to solve the problem. This value was already increased 
in D92715  but I assume this has some impact 
on the on-disk format.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153111

Files:
  clang/include/clang/Serialization/ASTBitCodes.h


Index: clang/include/clang/Serialization/ASTBitCodes.h
===
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -1106,7 +1106,7 @@
 ///
 /// Type IDs for non-predefined types will start at
 /// NUM_PREDEF_TYPE_IDs.
-const unsigned NUM_PREDEF_TYPE_IDS = 300;
+const unsigned NUM_PREDEF_TYPE_IDS = 500;
 
 /// Record codes for each kind of type.
 ///


Index: clang/include/clang/Serialization/ASTBitCodes.h
===
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -1106,7 +1106,7 @@
 ///
 /// Type IDs for non-predefined types will start at
 /// NUM_PREDEF_TYPE_IDs.
-const unsigned NUM_PREDEF_TYPE_IDS = 300;
+const unsigned NUM_PREDEF_TYPE_IDS = 500;
 
 /// Record codes for each kind of type.
 ///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152070: [2/11][Clang][RISCV] Expand all variants of RVV intrinsic tuple types

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

>> Can you check if clang crashes for you?
>
> I am seeing messages related to builtin type, but they do not seem related to 
> RVV builtin types.
>
>   $ touch t.c
>   $ clang -cc1 -triple riscv64 -w -emit-pch -o test.pch t.c
>   $ clang -cc1 -triple riscv64 -w -x c -include-pch test.pch -ast-dump-all 
> /dev/null
>   TranslationUnitDecl 0x60f3f88 <>  
> 
>   |-TypedefDecl 0x60f4b78 <>  implicit __int128_t 
> '__int128'
>   | `-BuiltinType 0x60f4520 '__int128'
>   |-TypedefDecl 0x60f4be8 <>  implicit 
> __uint128_t 'unsigned __int128'
>   | `-BuiltinType 0x60f4540 'unsigned __int128'
>   |-TypedefDecl 0x60f4ae8 <>  implicit 
> __NSConstantString 'struct __NSConstantString_tag'
>   | `-RecordType 0x60f48c0 'struct __NSConstantString_tag' imported
>   |   `-Record 0x60f4828 '__NSConstantString_tag'
>   `-TypedefDecl 0x60f4c58 <>  implicit 
> __builtin_va_list 'void *'
> `-PointerType 0x60f47a0 'void *'
>   `-BuiltinType 0x60f3fe0 'void'

Thanks @eopXD.

What I've seen is that the tst stops crashing for me if I disable all the types 
starting from line 218 in file `clang/include/clang/Basic/RISCVVTypes.def`.

When it doesn't crash, in my case all the registered RVV types are shown in the 
output

  TranslationUnitDecl 0x14647eb8 <>  

  |-TypedefDecl 0x14649e00 <>  implicit __int128_t 
'__int128'
  | `-BuiltinType 0x14648480 '__int128'
  |-TypedefDecl 0x14649e70 <>  implicit __uint128_t 
'unsigned __int128'
  | `-BuiltinType 0x146484a0 'unsigned __int128'
  |-TypedefDecl 0x14649d68 <>  implicit 
__NSConstantString 'struct __NSConstantString_tag'
  | `-RecordType 0x14649b40 'struct __NSConstantString_tag' imported
  |   `-Record 0x14649aa0 '__NSConstantString_tag'
  |-TypedefDecl 0x14649ed0 <>  imported implicit 
__rvv_int8mf8_t '__rvv_int8mf8_t'
  | `-BuiltinType 0x14648640 '__rvv_int8mf8_t'
  |-TypedefDecl 0x14649f58 <>  imported implicit 
__rvv_int8mf4_t '__rvv_int8mf4_t'
  | `-BuiltinType 0x14648660 '__rvv_int8mf4_t'
  |-TypedefDecl 0x14649fe0 <>  imported implicit 
__rvv_int8mf2_t '__rvv_int8mf2_t'
  | `-BuiltinType 0x14648680 '__rvv_int8mf2_t'
  |-TypedefDecl 0x1464a068 <>  imported implicit 
__rvv_int8m1_t '__rvv_int8m1_t'
  | `-BuiltinType 0x146486a0 '__rvv_int8m1_t'
  |-TypedefDecl 0x1464a0f0 <>  imported implicit 
__rvv_int8m2_t '__rvv_int8m2_t'
  | `-BuiltinType 0x146486c0 '__rvv_int8m2_t'
  |-TypedefDecl 0x1464a178 <>  imported implicit 
__rvv_int8m4_t '__rvv_int8m4_t'
  …

Curious that you don't observe this. I'll keep investigating.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152070

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

[PATCH] D152070: [2/11][Clang][RISCV] Expand all variants of RVV intrinsic tuple types

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

In D152070#4425358 , @eopXD wrote:

> In D152070#4425318 , @rogfer01 
> wrote:
>
>> In D152070#4421004 , 
>> @DavidSpickett wrote:
>>
>>> FYI after this change:
>>>
>>>   Building CXX object 
>>> tools/lldb/sou...luginTypeSystemClang.dir/TypeSystemClang.cpp.o
>>>   
>>> /home/david.spickett/llvm-project/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:4843:13:
>>>  warning: 225 enumeration values not handled in switch: 'RvvInt8mf8x2', 
>>> 'RvvInt8mf8x3', 'RvvInt8mf8x4'... [-Wswitch]
>>>   switch (llvm::cast(qual_type)->getKind()) {
>>>   ^~~~
>>>
>>> lldb doesn't do anything with RVV yet, so you can likely just add all the 
>>> names to the existing block of RVV stuff that just `break`s at the end.
>>
>> This might be the cause to a crash I'm observing and that bisect pointed to.
>>
>> Reproducer:
>>
>>   $ touch t.c # empty file
>>   $ clang -cc1 -triple riscv64 -w -emit-pch -o test.pch t.c
>>   $ clang -cc1 -triple riscv64 -w -x c -include-pch test.pch -ast-dump-all 
>> /dev/null
>
> Warning (and possibly the error you got) should be fixed now through 
> https://reviews.llvm.org/D152922.

Sorry @eopXD I confused everyone by linking the clang issue with that of lldb 
and they are unrelated. Apologies.

Can you check if clang crashes for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152070

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

[PATCH] D152137: [4/6][Clang][RISCV] Replace strided segment store with tuple type interfaces

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 accepted this revision.
rogfer01 added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: wangpc.

LGTM. Thanks @eopXD


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152137

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

[PATCH] D152070: [2/11][Clang][RISCV] Expand all variants of RVV intrinsic tuple types

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.
Herald added a subscriber: wangpc.

In D152070#4421004 , @DavidSpickett 
wrote:

> FYI after this change:
>
>   Building CXX object 
> tools/lldb/sou...luginTypeSystemClang.dir/TypeSystemClang.cpp.o
>   
> /home/david.spickett/llvm-project/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:4843:13:
>  warning: 225 enumeration values not handled in switch: 'RvvInt8mf8x2', 
> 'RvvInt8mf8x3', 'RvvInt8mf8x4'... [-Wswitch]
>   switch (llvm::cast(qual_type)->getKind()) {
>   ^~~~
>
> lldb doesn't do anything with RVV yet, so you can likely just add all the 
> names to the existing block of RVV stuff that just `break`s at the end.

This might be the cause to a crash I'm observing and that bisect pointed to.

Reproducer:

  $ touch t.c # empty file
  $ clang -cc1 -triple riscv64 -w -emit-pch -o test.pch t.c
  $ clang -cc1 -triple riscv64 -w -x c -include-pch test.pch -ast-dump-all 
/dev/null


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152070

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

[PATCH] D152139: [6/6][Clang][RISCV] Replace indexed segment store with tuple type interfaces

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 accepted this revision.
rogfer01 added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: wangpc.

LGTM. Thanks @eopXD


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152139

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

[PATCH] D152138: [5/6][Clang][RISCV] Replace indexed segment load with tuple type interfaces

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 accepted this revision.
rogfer01 added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: wangpc.

LGTM. Thanks @eopXD


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152138

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

[PATCH] D153008: [RISCV] Allow slash-star comments in instruction operands

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Hi @abel-bernabeu, I did a quick experiment replacing all the calls to 
`getLexer().Lex()` with `getParser().Lex()` and now your testcase is accepted 
and looks like this in the output

  f2:
addisp, sp, -32
sd  ra, 24(sp)
sd  s0, 16(sp)
addis0, sp, 32
#APP
lui a0, 1   # this is fine  # this should also be fine 
addiw   a0, a0, 564
add zero, a0, a0
  
#NO_APP

I understand this is the best we can do within assembly syntax.

I'm not suggesting we should change all of them. There are ~20 occurrences of 
`getLexer.Lex()` so I think it should not be too onerous to review each one.

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153008

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

[PATCH] D152135: [2/6][Clang][RISCV] Replace unit-stride segment store with tuple type interfaces

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Just to confirm, though: seeing those `COERCE`, I assume the underlying calling 
convention (which unfortunately it not 100% transparent to `clang` at this 
point) is the same as passing in order the components of the tuple, right?

(Apologies if this was already discussed elsewhere)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152135

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

[PATCH] D152136: [3/6][Clang][RISCV] Replace strided segment load with tuple type interfaces

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

LGTM. Thanks @eopXD


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152136

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

[PATCH] D152135: [2/6][Clang][RISCV] Replace unit-stride segment store with tuple type interfaces

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

LGTM. Thanks @eopXD


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152135

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

[PATCH] D152134: [1/6][Clang][RISCV] Replace unit-stride (fault-first) segment load with tuple type interfaces

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

LGTM. Thanks @eopXD


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152134

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

[PATCH] D126743: [RISCV][Clang] Add tests for all supported policy functions. (NFC)

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

This is a huge number of test changes so I have checked a subset of files but 
the changes seems reasonable, as I imagine they're generated mechanically.

We seem to be internally consistent, the following pairs generate the same IR

- `_m` and `_tumu` versions
- unsuffixed versions and `_ta` versions

So no objection on landing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126743

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

[PATCH] D126741: [RISCV][Clang] Refactor RISCVVEmitter. (NFC)

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

LGTM. Thanks @khchen !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126741

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

[PATCH] D126740: [RISCV][Clang] Refactor and rename rvv intrinsic related stuff. (NFC)

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

LGTM. Thanks @khchen !

(I was a bit confused by the refactoring of `computeBuiltinTypes` but it is 
definitely better this way)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126740

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

[PATCH] D124186: [RISCV] Fix incorrect policy implement for unmasked vslidedown and vslideup.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

LGTM. Thanks @khchen !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124186

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

[PATCH] D120870: [RISCV][NFC] Refine and refactor RISCVVEmitter and riscv_vector.td.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

Thanks for the cleanup @khchen !

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120870

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

[PATCH] D120228: [RISCV] Add policy operand for masked compare and vmsbf/vmsif/vmsof IR intrinsics.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

LGTM. Thanks @khchen !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120228

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

[PATCH] D120227: [RISCV] Add policy operand for masked vid and viota IR intrinsics.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

LGTM. Thanks @khchen !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120227

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

[PATCH] D119686: [RISCV] Add the passthru operand for nomask vadc/vsbc/vmerge/vfmerge IR intrinsics.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

LGTM. Thanks @khchen!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119686

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

[PATCH] D119688: [RISCV] Add the passthru operand for vmv.vv/vmv.vx/vfmv.vf IR intrinsics.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

LGTM. Thanks @khchen !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119688

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

[PATCH] D119541: [RISCV] Fix RISCVTargetInfo::initFeatureMap, add non-ISA features back after implication

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added inline comments.



Comment at: clang/test/Driver/riscv-default-features.c:4
+// RV32: "target-features"="+a,+c,+m,+relax,-save-restore"
+// RV64: "target-features"="+64bit,+a,+c,+m,+relax,-save-restore"
+

I think we may be missing are missing a `RUN` line for this case, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119541

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

[PATCH] D117989: [RISCV] Add the passthru operand for RVV nomask binary intrinsics.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

Looks reasonable to me. Thans @khchen!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117989

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

[PATCH] D112398: [RISCV] Add ABI testing for Float16.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

I'm curious about how we handle `_Float16` here.

My understanding from the psabi 

 and the Zfh draft 
, don't 
we want under `Zfh` + `ilp32f` to pass the float16 directly in a FP register?

So for a case like this

  typedef _Float16 FP;
  typedef struct A { FP a; FP b; } A;
  
  void foo(A);
  
  void bar()
  {
  A a = {1.0, 2.0};
  foo(a);
  }

I'd expect  `Zfh` +  ilp32` to pass an `i32` (which we already do)

  define dso_local void @bar() local_unnamed_addr #0 {
tail call void @foo(i32 1073757184) #2
ret void
  }

and `Zfh` + `ilp32f` (and I understand `ilp32d` too) do

  define dso_local void @bar() local_unnamed_addr #0 {
tail call void @foo(half 1.0e+0.0, half 2.0e+0.0)
ret void
  }

Perhaps it was already decided against using FP regs for `_Float16`?

I think a similar consideration can be done for `rv64` + `Zfh` and `lp64` / 
`lp64f` / `lp64d`, the last two using FP registers.

If this is the case, then I understand a parameter like

  typedef struct A { _Float16 a; float b; } A;

would also be passed as two FP registers in `ilp32f`, `ilp32d`, `lp64f` and 
`lp64d`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112398

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

[PATCH] D112102: [RISCV] Reduce the number of RISCV vector builtins by an order of magnitude.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Looks good to me too. Thanks a lot @craig.topper !


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112102

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

[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

In D111617#3060377 , @HsiangKai wrote:

> Although it reduces the header size, this patch will increase the binary size 
> of clang.

Options we can consider to mitigate this:

- See if we can be more economical reprenting the table itself
- Store it in the binary in a format unsuitable for lookups (e.g. compressed) 
and then transform it (in memory) the first time is used so queries can be 
solved in a reasonable amount of time.

> Or we should go back to think about the design of RVV intrinsics?

Certainly the explosion of cases is an issue.

Maybe an alternative is using overloaded builtins (similar to what atomic 
builtins can do when the type is not specified), but I understand this means 
implementing the typechecking itself. Which perhaps it can be generated too.

This looks like it might bring a lot of reduction in cases and tables. Corner 
cases will appear when the arguments of a call are not enough to determine the 
precise intrinsic desired, e.g. loads as they are currently defined would be 
ambiguous between different LMUL types (though there may be ways to mitigate 
this, e.g. overload only within LMULs).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617

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

[PATCH] D111617: [RISCV] Lazily add RVV C intrinsics.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added inline comments.



Comment at: clang/lib/Sema/SemaLookup.cpp:923
+
+  const RVVIntrinsicInfo *Intrinsic = std::find_if(
+  std::begin(RVVIntrinsicInfos), std::end(RVVIntrinsicInfos),

Not for this patch: I think this table may be a bit large so all lookups 
(including those that will fail) will be slower after a `#pragma riscv 
intrinsic vector` is found.

Filtering them as fast as possible (looking at the spec shows that currently 
all RVV intrinsics start with `v`) or using some hash table (if too difficult 
to build at compile time we could build it the first time we get here?) might 
be something we want to do.



Comment at: clang/lib/Sema/SemaLookup.cpp:1011
 
+  if (PP.getPredefines() == "#define __riscv_pragma_vector_intrinsics") {
+const TargetInfo  = Context.getTargetInfo();

This seems a bit fragile if there are more predefines than just this one. I 
understand the intent is to avoid looking up the RVV builtin every time, only 
do that if we have found the pragma, right?

Several pragma handlers receive a reference to `Sema` (in an object called 
`Action` or `Actions`) and then they notify `Sema` (via a member function that 
would have to be added to it) about having parsed the pragma. That could be 
used to set some flag to true in `Sema` itself and also emit diagnostics if we 
want (e.g. what if the pragma is used twice? can it be used anywhere?).

Do you think this would be workable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111617

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

[PATCH] D107666: [OpenMP] Fix accidental reuse of VLA size

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbfb77364d0be: [OpenMP] Fix accidental reuse of VLA size 
(authored by rogfer01).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107666

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/vla_iterator_cache_bug.c


Index: clang/test/OpenMP/vla_iterator_cache_bug.c
===
--- /dev/null
+++ clang/test/OpenMP/vla_iterator_cache_bug.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-linux-gnu \
+// RUN:   -emit-llvm %s -o - | FileCheck %s
+
+// expected-no-diagnostics
+
+extern int bounds1(int);
+extern int bounds2(int);
+
+extern void fun2(int n, int *a, int *b);
+extern void fun3(int n, int *a, int *b);
+
+void fun1(int n, int *a, int *b)
+{
+#pragma omp task depend(iterator(j = 0 : bounds1(n)), in : a[b[j]])
+  {
+fun2(n, a, b);
+  }
+// CHECK: alloca %struct.kmp_depend_info, i64 [[FIRST_VLA:%.*]], align 16
+
+#pragma omp task depend(iterator(j = 0 : bounds2(n)), in : a[b[j]])
+  {
+fun3(n, a, b);
+  }
+// CHECK-NOT: alloca %struct.kmp_depend_info, i64 [[FIRST_VLA]], align 16
+}
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -4401,14 +4401,14 @@
 if (NumOfElements) {
   NumOfElements = CGF.Builder.CreateNUWAdd(
   llvm::ConstantInt::get(CGF.SizeTy, NumAffinities), NumOfElements);
-  OpaqueValueExpr OVE(
+  auto *OVE = new (C) OpaqueValueExpr(
   Loc,
   C.getIntTypeForBitwidth(C.getTypeSize(C.getSizeType()), 
/*Signed=*/0),
   VK_PRValue);
-  CodeGenFunction::OpaqueValueMapping OpaqueMap(CGF, ,
+  CodeGenFunction::OpaqueValueMapping OpaqueMap(CGF, OVE,
 
RValue::get(NumOfElements));
   KmpTaskAffinityInfoArrayTy =
-  C.getVariableArrayType(KmpTaskAffinityInfoTy, , 
ArrayType::Normal,
+  C.getVariableArrayType(KmpTaskAffinityInfoTy, OVE, ArrayType::Normal,
  /*IndexTypeQuals=*/0, SourceRange(Loc, Loc));
   // Properly emit variable-sized array.
   auto *PD = ImplicitParamDecl::Create(C, KmpTaskAffinityInfoArrayTy,
@@ -4899,13 +4899,13 @@
   NumOfElements =
   CGF.Builder.CreateNUWAdd(NumOfRegularWithIterators, NumOfElements);
 }
-OpaqueValueExpr OVE(Loc,
-C.getIntTypeForBitwidth(/*DestWidth=*/64, 
/*Signed=*/0),
-VK_PRValue);
-CodeGenFunction::OpaqueValueMapping OpaqueMap(CGF, ,
+auto *OVE = new (C) OpaqueValueExpr(
+Loc, C.getIntTypeForBitwidth(/*DestWidth=*/64, /*Signed=*/0),
+VK_PRValue);
+CodeGenFunction::OpaqueValueMapping OpaqueMap(CGF, OVE,
   RValue::get(NumOfElements));
 KmpDependInfoArrayTy =
-C.getVariableArrayType(KmpDependInfoTy, , ArrayType::Normal,
+C.getVariableArrayType(KmpDependInfoTy, OVE, ArrayType::Normal,
/*IndexTypeQuals=*/0, SourceRange(Loc, Loc));
 // CGF.EmitVariablyModifiedType(KmpDependInfoArrayTy);
 // Properly emit variable-sized array.


Index: clang/test/OpenMP/vla_iterator_cache_bug.c
===
--- /dev/null
+++ clang/test/OpenMP/vla_iterator_cache_bug.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-linux-gnu \
+// RUN:   -emit-llvm %s -o - | FileCheck %s
+
+// expected-no-diagnostics
+
+extern int bounds1(int);
+extern int bounds2(int);
+
+extern void fun2(int n, int *a, int *b);
+extern void fun3(int n, int *a, int *b);
+
+void fun1(int n, int *a, int *b)
+{
+#pragma omp task depend(iterator(j = 0 : bounds1(n)), in : a[b[j]])
+  {
+fun2(n, a, b);
+  }
+// CHECK: alloca %struct.kmp_depend_info, i64 [[FIRST_VLA:%.*]], align 16
+
+#pragma omp task depend(iterator(j = 0 : bounds2(n)), in : a[b[j]])
+  {
+fun3(n, a, b);
+  }
+// CHECK-NOT: alloca %struct.kmp_depend_info, i64 [[FIRST_VLA]], align 16
+}
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -4401,14 +4401,14 @@
 if (NumOfElements) {
   NumOfElements = CGF.Builder.CreateNUWAdd(
   llvm::ConstantInt::get(CGF.SizeTy, NumAffinities), NumOfElements);
-  OpaqueValueExpr OVE(
+  auto *OVE = new (C) OpaqueValueExpr(
   Loc,
   C.getIntTypeForBitwidth(C.getTypeSize(C.getSizeType()), /*Signed=*/0),
   VK_PRValue);
-  CodeGenFunction::OpaqueValueMapping OpaqueMap(CGF, ,
+  CodeGenFunction::OpaqueValueMapping 

[PATCH] D107666: [OpenMP] Fix accidental reuse of VLA size

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Thanks for the prompt review @ABataev! I'll push this shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107666

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

[PATCH] D107666: [OpenMP] Fix accidental reuse of VLA size

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 created this revision.
rogfer01 added a reviewer: ABataev.
Herald added subscribers: guansong, yaxunl.
rogfer01 requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

We were using an `OpaqueValueExpr` allocated on the stack to store the size of 
a VLA. Because the `VLASizeMap` in `CodegenFunction` uses the address of the 
expression to avoid recomputing VLAs, we were accidentally reusing an earlier 
`llvm::Value`. This led to invalid LLVM IR.

After this change, we allocate the `OpaqueValueExpr` in `ASTContext` rather 
than in the stack. This is a temporary solution until `VLASizeMap` can be 
pushed and popped based on the context.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107666

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/vla_iterator_cache_bug.c


Index: clang/test/OpenMP/vla_iterator_cache_bug.c
===
--- /dev/null
+++ clang/test/OpenMP/vla_iterator_cache_bug.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-linux-gnu \
+// RUN:   -emit-llvm %s -o - | FileCheck %s
+
+// expected-no-diagnostics
+
+extern int bounds1(int);
+extern int bounds2(int);
+
+extern void fun2(int n, int *a, int *b);
+extern void fun3(int n, int *a, int *b);
+
+void fun1(int n, int *a, int *b)
+{
+#pragma omp task depend(iterator(j = 0 : bounds1(n)), in : a[b[j]])
+  {
+fun2(n, a, b);
+  }
+// CHECK: alloca %struct.kmp_depend_info, i64 [[FIRST_VLA:%.*]], align 16
+
+#pragma omp task depend(iterator(j = 0 : bounds2(n)), in : a[b[j]])
+  {
+fun3(n, a, b);
+  }
+// CHECK-NOT: alloca %struct.kmp_depend_info, i64 [[FIRST_VLA]], align 16
+}
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -4400,14 +4400,14 @@
 if (NumOfElements) {
   NumOfElements = CGF.Builder.CreateNUWAdd(
   llvm::ConstantInt::get(CGF.SizeTy, NumAffinities), NumOfElements);
-  OpaqueValueExpr OVE(
+  auto *OVE = new (C) OpaqueValueExpr(
   Loc,
   C.getIntTypeForBitwidth(C.getTypeSize(C.getSizeType()), 
/*Signed=*/0),
   VK_PRValue);
-  CodeGenFunction::OpaqueValueMapping OpaqueMap(CGF, ,
+  CodeGenFunction::OpaqueValueMapping OpaqueMap(CGF, OVE,
 
RValue::get(NumOfElements));
   KmpTaskAffinityInfoArrayTy =
-  C.getVariableArrayType(KmpTaskAffinityInfoTy, , 
ArrayType::Normal,
+  C.getVariableArrayType(KmpTaskAffinityInfoTy, OVE, ArrayType::Normal,
  /*IndexTypeQuals=*/0, SourceRange(Loc, Loc));
   // Properly emit variable-sized array.
   auto *PD = ImplicitParamDecl::Create(C, KmpTaskAffinityInfoArrayTy,
@@ -4898,13 +4898,13 @@
   NumOfElements =
   CGF.Builder.CreateNUWAdd(NumOfRegularWithIterators, NumOfElements);
 }
-OpaqueValueExpr OVE(Loc,
-C.getIntTypeForBitwidth(/*DestWidth=*/64, 
/*Signed=*/0),
-VK_PRValue);
-CodeGenFunction::OpaqueValueMapping OpaqueMap(CGF, ,
+auto *OVE = new (C) OpaqueValueExpr(
+Loc, C.getIntTypeForBitwidth(/*DestWidth=*/64, /*Signed=*/0),
+VK_PRValue);
+CodeGenFunction::OpaqueValueMapping OpaqueMap(CGF, OVE,
   RValue::get(NumOfElements));
 KmpDependInfoArrayTy =
-C.getVariableArrayType(KmpDependInfoTy, , ArrayType::Normal,
+C.getVariableArrayType(KmpDependInfoTy, OVE, ArrayType::Normal,
/*IndexTypeQuals=*/0, SourceRange(Loc, Loc));
 // CGF.EmitVariablyModifiedType(KmpDependInfoArrayTy);
 // Properly emit variable-sized array.


Index: clang/test/OpenMP/vla_iterator_cache_bug.c
===
--- /dev/null
+++ clang/test/OpenMP/vla_iterator_cache_bug.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-linux-gnu \
+// RUN:   -emit-llvm %s -o - | FileCheck %s
+
+// expected-no-diagnostics
+
+extern int bounds1(int);
+extern int bounds2(int);
+
+extern void fun2(int n, int *a, int *b);
+extern void fun3(int n, int *a, int *b);
+
+void fun1(int n, int *a, int *b)
+{
+#pragma omp task depend(iterator(j = 0 : bounds1(n)), in : a[b[j]])
+  {
+fun2(n, a, b);
+  }
+// CHECK: alloca %struct.kmp_depend_info, i64 [[FIRST_VLA:%.*]], align 16
+
+#pragma omp task depend(iterator(j = 0 : bounds2(n)), in : a[b[j]])
+  {
+fun3(n, a, b);
+  }
+// CHECK-NOT: alloca %struct.kmp_depend_info, i64 [[FIRST_VLA]], align 16
+}
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ 

[PATCH] D106044: [RISCV] Update to vcpop.m, vlm.v and vsm.v according to v1.0-rc1.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

I'm confused because the PDF at 
https://github.com/riscv/riscv-v-spec/releases/tag/v1.0-rc1  doesn't seem to 
describe  `vcpop.m`.

I can see this has changed in the ToT 
https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#changes-from-v1-0-rc1

Perhaps this will be part of a future 1.0-rc2?




Comment at: llvm/test/CodeGen/RISCV/rvv/vse1-rv64.ll:12
+; CHECK-NEXT:sd ra, 8(sp) # 8-byte Folded Spill
+; CHECK-NEXT:call llvm.riscv.vse1.nxv1i1@plt
+; CHECK-NEXT:ld ra, 8(sp) # 8-byte Folded Reload

Is this expected?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106044

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

[PATCH] D103527: [Clang][RISCV] Implement vlseg and vlsegff.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Do you think you can split `vlseg` and `vlseg_ff` tests in two different files? 
So we (potentially) reduce the test latency by half in multicore systems.

Other than this, LGTM.

We're a bit split in https://github.com/riscv/rvv-intrinsic-doc/issues/95 
However there are mechanisms to assist in any transition should we be able to 
implement in the future alternative interfaces that we prefer over these ones.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103527

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

[PATCH] D100821: [RISCV] Implement the vmmv.m/vmnot.m builtin.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

In D100821#2710998 , @HsiangKai wrote:

> Rebase.

Thanks, I had missed that and my comment above was wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100821

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

[PATCH] D100821: [RISCV] Implement the vmmv.m/vmnot.m builtin.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

I can reproduce the crash above in my computer, so something is definitely off.

Looks like we emit this

  case RISCV::BI__builtin_rvv_vmmv_m_b8:
  case RISCV::BI__builtin_rvv_vmmv_m_b4:
  case RISCV::BI__builtin_rvv_vmmv_m_b2:
  case RISCV::BI__builtin_rvv_vmmv_m_b1:
  case RISCV::BI__builtin_rvv_vmmv_m_b16:   
  case RISCV::BI__builtin_rvv_vmmv_m_b32:   
  case RISCV::BI__builtin_rvv_vmmv_m_b64:   
  case RISCV::BI__builtin_rvv_vmand_mm_b8:  
  case RISCV::BI__builtin_rvv_vmand_mm_b4:  
  case RISCV::BI__builtin_rvv_vmand_mm_b2:  
  case RISCV::BI__builtin_rvv_vmand_mm_b1:  
  case RISCV::BI__builtin_rvv_vmand_mm_b16: 
  case RISCV::BI__builtin_rvv_vmand_mm_b32: 
  case RISCV::BI__builtin_rvv_vmand_mm_b64: 
ID = Intrinsic::riscv_vmand;
IntrinsicTypes = {ResultType, Ops[2]->getType()};   
break;  

But `Ops` has size 2 so this is accessing past the end. I observed the crash 
with `clang::RISCV::BI__builtin_rvv_vmmv_m_b1` but I assume it happens with the 
other ones as well.

I think the description of the builtin should be `def : RVVBuiltin<"m", "m", 
type_range>;` as these are like unary operations, aren't they?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100821

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

[PATCH] D99741: [RISCV][Clang] Add some RVV Floating-Point intrinsic functions. (vfclass, vfmerge, vfrec7, vfrsqrt7, vfsqrt)

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

I've left a PoC of my approach here https://reviews.llvm.org/D100529 in case we 
can use that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99741

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

[PATCH] D99741: [RISCV][Clang] Add some RVV Floating-Point intrinsic functions. (vfclass, vfmerge, vfrec7, vfrsqrt7, vfsqrt)

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

In D99741#2690156 , @craig.topper 
wrote:

> In D99741#2690124 , @thakis wrote:
>
>> One of the ten commits that landed here 
>> (https://github.com/llvm/llvm-project/compare/a3bfddbb6a27...59d5b8c27b43), 
>> this being one of them, slowed down check-clang by over 20%: 
>> https://bugs.llvm.org/show_bug.cgi?id=49962
>>
>> Since recovering test runtime regressions is very difficult, it's probably a 
>> good idea to revert this first and then reland in smaller chunks to see 
>> which of the commits causes the problem (?)
>
> The fact that we're running the optimization pipeline in these tests might be 
> to blame. Might also be that riscv_vector.h is currently about 71000 lines 
> which probably isn't quick to parse.

I hacked the `RISCVVEmitter.cpp` file to emit a `riscv_vector_testing.h` that 
wraps all the definitions with macros like `RVV_TEST_vadd` and 
`RVV_TEST_vadd_mask` (I used the `IRName` as it was easier for the experiment).

The testing header looks like this

  #if defined(RVV_TEST_vadd)
   
  __rvv_overloaded vint8m1_t vadd(vint8m1_t op0, vint8m1_t op1, size_t op2){
   
return vadd_vv_i8m1(op0, op1, op2); 
   
  } 
   
  #endif // RVV_TEST_vadd   
   

   
  #if defined(RVV_TEST_vadd_mask)   
   
  __rvv_overloaded vint8m1_t vadd(vbool8_t op0, vint8m1_t op1, vint8m1_t op2, 
vint8m1_t op3, size_t op4){
return vadd_vv_i8m1_m(op0, op1, op2, op3, op4); 
   
  } 
   
  #endif // RVV_TEST_vadd_mask  
   

In a release build (in a machine that is not very fast) looks like this:

  $ ./bin/llvm-lit --time-test -sv 
../llvm-src/clang/test/CodeGen/RISCV/rvv-intrinsics/vadd.c 
../llvm-src/clang/test/CodeGen/RISCV/rvv-intrinsics/vadd_fast.c 
  llvm-lit: 
/home/rferrer/fio/upstream/llvm-src/llvm/utils/lit/lit/llvm/config.py:428: 
note: using clang: /home/rferrer/fio/upstream/llvm-build-release/bin/clang
  Slowest Tests:
  --
  8.91s: Clang :: CodeGen/RISCV/rvv-intrinsics/vadd.c
  2.11s: Clang :: CodeGen/RISCV/rvv-intrinsics/vadd_fast.c

Where `vadd.c` is the current file and `vadd_fast.c` is the same as vadd.c with 
this change

  --- vadd.c2021-04-15 06:51:30.554996630 +
  +++ vadd_fast.c   2021-04-15 07:00:38.684074973 +
  @@ -8,7 +8,9 @@
   // RUN:   -Werror -Wall -o - %s -S >/dev/null 2>&1 | FileCheck 
--check-prefix=ASM --allow-empty %s
   
   // ASM-NOT: warning
  -#include 
  +#define RVV_TEST_vadd
  +#define RVV_TEST_vadd_mask
  +#include 
   
   // CHECK-RV32-LABEL: @test_vadd_vv_i8mf8(
   // CHECK-RV32-NEXT:  entry:

This seems to suggest that making sure the tested header contains only the 
necessary will speed the testing up significantly.

We would want to make sure `riscv_vector.h` and `riscv_vector_testing.h` are 
equivalent when all the macros are enabled. I guess such a test is feasible if 
we preprocess the testing header with all the `RVV_TEST_nnn` macros enabled and 
then we do some form of textual comparison (which will likely have to ignore 
whitespace). This way we can test `riscv_vector.h` using its proxy 
`riscv_vector_testing.h`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99741

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

[PATCH] D97264: [RISCV] Define types for Zvlsseg.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:1486
+  BuiltinType::Kind K, unsigned NF) {
+  auto TypeIter = llvm::find_if(Types, [](Type *Ty) {
+ if (Ty->isBuiltinType()) {

I'm confused by this lookup.

If you add `RvvInt8mf8Ty` in the macros above will you be able to use it for 
types such as `RvvInt8mf8x2Ty`, `RvvInt8mf8x3Ty`, etc. without having to search 
it in all the types?

AFAICT `ElemId` seems only used in this case so perhaps we can use the "name of 
the field of ASTContext" (a `CanQualType`) directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97264

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

[PATCH] D97264: [RISCV] Define types for Zvlsseg.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

I was under the impression we didn't want to use class-member access syntax for 
vector tuples (see 
https://github.com/riscv/rvv-intrinsic-doc/issues/17#issuecomment-628998077 ) 
so we don't need a record type, do we?

Perhaps it is possible to model them like opaque entities similar to what we do 
with RVV vectors where they are expanded in `CodegenTypes.cpp`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97264

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

[PATCH] D99593: [Clang][RISCV] Implement vlseg builtins.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

This is just a suggestion, feel free to ignore: a sequence of `T`s was easy to 
parse for the prototype but may be want to consider something like `T3v` rather 
than `TTTv`. I think it would simplify the `TString` tblgen class and those 
`std::string(N, 'T')`.




Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:594
   auto PType = Transformer.back();
+  static unsigned NFCount = 0;
   switch (PType) {

Was this meant to be `static` here?



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:714
+
+  if (PType == 'v' && NFCount > 0) {
+IsTuple = true;

We say `Tv` is invalid, looks like we should check this here and 
`PrintFatalError`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99593

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

[PATCH] D98388: [RISCV][Clang] Add RVV vle/vse intrinsic functions.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Overall LGTM. Thanks @khchen!




Comment at: clang/include/clang/Basic/riscv_vector.td:175
+  // builtin to C/C++. It is parameter of the unmasked version without VL
+  // operand.
+  list PermuteOperands = [];

Not sure if we want to clarify that when this list is empty, the permutation is 
assumed to be equivalent to `[0, 1, 2, 3, ...]`.



Comment at: clang/include/clang/Basic/riscv_vector.td:224
+IntrinsicTypes = {ResultType, Ops[1]->getType()};
+Ops[0] = Builder.CreateBitCast(Ops[0],
+llvm::PointerType::getUnqual(ResultType)); }],

I think you may want to indent this, and then lines 228 and 248 like you 
indented line 252.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:689
+  skew = 1;
+for (unsigned i = 0; i < PermuteOperands.size(); ++i) {
+  if (i != PermuteOperands[i])

These are only suggestions of sanity checks we could do here:
- I understand `PermuteOperand.size()` should be `<=` than `CTypeOrder.size()`. 
- Also `PermuteOperands[i] + skew` should be `<` than `CTypeOrder.size()`. 
right?
- We could check the result is indeed a permutation (e.g. sorting a copy of 
`CTypeOrder` is equivalent to the iota above). This one might be expensive 
although the sequences are short, not sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98388

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

[PATCH] D80802: [RISCV] Upgrade RVV MC to v0.9.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added inline comments.



Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:796
 }
 return "";
   }

Minor nit that we missed in the patch of 0.8: can you replace this return with

```lang=cpp
llvm_unreachable();
```

as recommended in the second paragraph of 
https://llvm.org/docs/CodingStandards.html#don-t-use-default-labels-in-fully-covered-switches-over-enumerations

Ditto in `getSEWStr` above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80802



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

[PATCH] D69987: [RISCV] Assemble/Disassemble v-ext instructions.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

The patch as it stands now LGTM and I think it can be committed. Is there any 
objection remaining?

Any further comments @simoncook @asb?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69987



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

[PATCH] D69987: [RISCV] Assemble/Disassemble v-ext instructions.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

@HsiangKai: just to confirm and to avoid confusion for other reviewers.

> Assemble/disassemble RISC-V V extension instructions according to version 
> 0.8-draft-20191004 in https://github.com/riscv/riscv-v-spec/.

Is the patch against the spec published in 
https://github.com/riscv/riscv-v-spec/releases/tag/0.8 (which looks to me from 
20191214)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69987



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

[PATCH] D70799: [OpenMP] Lower taskyield using OpenMP IR Builder

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2bef1c0e5645: [OpenMP] Lower taskyield using OpenMP IR 
Builder (authored by rogfer01).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70799

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/taskyield_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  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
@@ -709,3 +709,20 @@
 return;
   emitTaskwaitImpl(Loc);
 }
+
+void OpenMPIRBuilder::emitTaskyieldImpl(const LocationDescription ) {
+  // Build call __kmpc_omp_taskyield(loc, thread_id, 0);
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocStr);
+  Constant *I32Null = ConstantInt::getNullValue(Int32);
+  Value *Args[] = {Ident, getOrCreateThreadID(Ident), I32Null};
+
+  Builder.CreateCall(getOrCreateRuntimeFunction(OMPRTL___kmpc_omp_taskyield),
+ Args);
+}
+
+void OpenMPIRBuilder::CreateTaskyield(const LocationDescription ) {
+  if (!updateToLocation(Loc))
+return;
+  emitTaskyieldImpl(Loc);
+}
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -170,6 +170,7 @@
 __OMP_RTL(__kmpc_global_thread_num, false, Int32, IdentPtr)
 __OMP_RTL(__kmpc_fork_call, true, Void, IdentPtr, Int32, ParallelTaskPtr)
 __OMP_RTL(__kmpc_omp_taskwait, false, Int32, IdentPtr, Int32)
+__OMP_RTL(__kmpc_omp_taskyield, false, Int32, IdentPtr, Int32, Int32)
 __OMP_RTL(__kmpc_push_num_threads, false, Void, IdentPtr, Int32,
   /* Int */ Int32)
 __OMP_RTL(__kmpc_push_proc_bind, false, Void, IdentPtr, Int32, /* Int */ Int32)
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -171,6 +171,7 @@
  Value *IfCondition, Value *NumThreads,
  omp::ProcBindKind ProcBind, bool IsCancellable);
 
+
   /// Generator for '#omp flush'
   ///
   /// \param Loc The location where the flush directive was encountered
@@ -181,6 +182,11 @@
   /// \param Loc The location where the taskwait directive was encountered.
   void CreateTaskwait(const LocationDescription& Loc);
 
+  /// Generator for '#omp taskyield'
+  ///
+  /// \param Loc The location where the taskyield directive was encountered.
+  void CreateTaskyield(const LocationDescription& Loc);
+
   ///}
 
 
@@ -251,6 +257,11 @@
   /// \param Loc The location at which the request originated and is fulfilled.
   void emitTaskwaitImpl(const LocationDescription );
 
+  /// Generate a taskyield runtime call.
+  ///
+  /// \param Loc The location at which the request originated and is fulfilled.
+  void emitTaskyieldImpl(const LocationDescription );
+
   /// Return the current thread ID.
   ///
   /// \param Ident The ident (ident_t*) describing the query origin.
Index: clang/test/OpenMP/taskyield_codegen.cpp
===
--- clang/test/OpenMP/taskyield_codegen.cpp
+++ clang/test/OpenMP/taskyield_codegen.cpp
@@ -1,6 +1,10 @@
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+//
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -3295,11 +3295,18 @@
 SourceLocation Loc) {
   if (!CGF.HaveInsertPoint())
 return;
-  // Build 

[PATCH] D70799: [OpenMP] Lower taskyield using OpenMP IR Builder

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 updated this revision to Diff 244611.
rogfer01 added a comment.

ChangeLog:

- Rebase


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

https://reviews.llvm.org/D70799

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/taskyield_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  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
@@ -709,3 +709,20 @@
 return;
   emitTaskwaitImpl(Loc);
 }
+
+void OpenMPIRBuilder::emitTaskyieldImpl(const LocationDescription ) {
+  // Build call __kmpc_omp_taskyield(loc, thread_id, 0);
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocStr);
+  Constant *I32Null = ConstantInt::getNullValue(Int32);
+  Value *Args[] = {Ident, getOrCreateThreadID(Ident), I32Null};
+
+  Builder.CreateCall(getOrCreateRuntimeFunction(OMPRTL___kmpc_omp_taskyield),
+ Args);
+}
+
+void OpenMPIRBuilder::CreateTaskyield(const LocationDescription ) {
+  if (!updateToLocation(Loc))
+return;
+  emitTaskyieldImpl(Loc);
+}
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -170,6 +170,7 @@
 __OMP_RTL(__kmpc_global_thread_num, false, Int32, IdentPtr)
 __OMP_RTL(__kmpc_fork_call, true, Void, IdentPtr, Int32, ParallelTaskPtr)
 __OMP_RTL(__kmpc_omp_taskwait, false, Int32, IdentPtr, Int32)
+__OMP_RTL(__kmpc_omp_taskyield, false, Int32, IdentPtr, Int32, Int32)
 __OMP_RTL(__kmpc_push_num_threads, false, Void, IdentPtr, Int32,
   /* Int */ Int32)
 __OMP_RTL(__kmpc_push_proc_bind, false, Void, IdentPtr, Int32, /* Int */ Int32)
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -171,6 +171,7 @@
  Value *IfCondition, Value *NumThreads,
  omp::ProcBindKind ProcBind, bool IsCancellable);
 
+
   /// Generator for '#omp flush'
   ///
   /// \param Loc The location where the flush directive was encountered
@@ -181,6 +182,11 @@
   /// \param Loc The location where the taskwait directive was encountered.
   void CreateTaskwait(const LocationDescription& Loc);
 
+  /// Generator for '#omp taskyield'
+  ///
+  /// \param Loc The location where the taskyield directive was encountered.
+  void CreateTaskyield(const LocationDescription& Loc);
+
   ///}
 
 
@@ -251,6 +257,11 @@
   /// \param Loc The location at which the request originated and is fulfilled.
   void emitTaskwaitImpl(const LocationDescription );
 
+  /// Generate a taskyield runtime call.
+  ///
+  /// \param Loc The location at which the request originated and is fulfilled.
+  void emitTaskyieldImpl(const LocationDescription );
+
   /// Return the current thread ID.
   ///
   /// \param Ident The ident (ident_t*) describing the query origin.
Index: clang/test/OpenMP/taskyield_codegen.cpp
===
--- clang/test/OpenMP/taskyield_codegen.cpp
+++ clang/test/OpenMP/taskyield_codegen.cpp
@@ -1,6 +1,10 @@
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+//
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -3295,11 +3295,18 @@
 SourceLocation Loc) {
   if (!CGF.HaveInsertPoint())
 return;
-  // Build call __kmpc_omp_taskyield(loc, thread_id, 0);
-  llvm::Value *Args[] = {
-  emitUpdateLocation(CGF, Loc), getThreadID(CGF, 

[PATCH] D69828: [OpenMP] Lower taskwait using OpenMP IR Builder

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa82f35e17621: [OpenMP] Lower taskwait using OpenMP IR 
Builder (authored by rogfer01).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69828

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/task_codegen.cpp
  clang/test/OpenMP/taskwait_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  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
@@ -691,3 +691,21 @@
return;
   emitFlush(Loc);
 }
+
+void OpenMPIRBuilder::emitTaskwaitImpl(const LocationDescription ) {
+  // Build call kmp_int32 __kmpc_omp_taskwait(ident_t *loc, kmp_int32
+  // global_tid);
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocStr);
+  Value *Args[] = {Ident, getOrCreateThreadID(Ident)};
+
+  // Ignore return result until untied tasks are supported.
+  Builder.CreateCall(getOrCreateRuntimeFunction(OMPRTL___kmpc_omp_taskwait),
+ Args);
+}
+
+void OpenMPIRBuilder::CreateTaskwait(const LocationDescription ) {
+  if (!updateToLocation(Loc))
+return;
+  emitTaskwaitImpl(Loc);
+}
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -169,6 +169,7 @@
 __OMP_RTL(__kmpc_flush, false, Void, IdentPtr)
 __OMP_RTL(__kmpc_global_thread_num, false, Int32, IdentPtr)
 __OMP_RTL(__kmpc_fork_call, true, Void, IdentPtr, Int32, ParallelTaskPtr)
+__OMP_RTL(__kmpc_omp_taskwait, false, Int32, IdentPtr, Int32)
 __OMP_RTL(__kmpc_push_num_threads, false, Void, IdentPtr, Int32,
   /* Int */ Int32)
 __OMP_RTL(__kmpc_push_proc_bind, false, Void, IdentPtr, Int32, /* Int */ Int32)
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -176,6 +176,11 @@
   /// \param Loc The location where the flush directive was encountered
   void CreateFlush(const LocationDescription );
 
+  /// Generator for '#omp taskwait'
+  ///
+  /// \param Loc The location where the taskwait directive was encountered.
+  void CreateTaskwait(const LocationDescription& Loc);
+
   ///}
 
 
@@ -241,6 +246,11 @@
FinalizationStack.back().DK == DK;
   }
 
+  /// Generate a taskwait runtime call.
+  ///
+  /// \param Loc The location at which the request originated and is fulfilled.
+  void emitTaskwaitImpl(const LocationDescription );
+
   /// Return the current thread ID.
   ///
   /// \param Ident The ident (ident_t*) describing the query origin.
Index: clang/test/OpenMP/taskwait_codegen.cpp
===
--- clang/test/OpenMP/taskwait_codegen.cpp
+++ clang/test/OpenMP/taskwait_codegen.cpp
@@ -1,6 +1,10 @@
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+//
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fopenmp-enable-irbuilder -x c++ -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp-simd -x c++ -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
Index: clang/test/OpenMP/task_codegen.cpp
===
--- clang/test/OpenMP/task_codegen.cpp
+++ clang/test/OpenMP/task_codegen.cpp
@@ -1,6 +1,10 @@
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+//
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fopenmp-enable-irbuilder -x c++ -emit-llvm %s -o - | FileCheck %s
+// RUN: 

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added inline comments.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:114
+
+// Add some known attributes to the outlined function.
+Function *OutlinedFn = Extractor.extractCodeRegion(CEAC);

This comment seems misplaced now.



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:654-655
+Instruction *PreFiniTI = PRegPreFiniBB->getTerminator();
+OuterFn->dump();
+PreFiniTI->dump();
+assert(PreFiniTI->getNumSuccessors() == 1 &&

I think you forgot to wrap these in a `LLVM_DEBUG`



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:663
 
-  return AfterIP;
+InsertPointTy ExitIP(UI->getParent(), UI->getParent()->end());
+UI->eraseFromParent();

This used to be called `AfterIP`. Probably the exact name is not that important 
but the unit tests still use `AfterIP` so I'd suggest to keep both names in 
sync.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74372



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

[PATCH] D73891: [RISCV] Support experimental/unratified extensions

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:50
 
+static bool isExperimentalExtension(StringRef Ext) {
+  // Currently 'b' is the only supported experimental extension

Suggestion: I think we can avoid these two functions (`isExperimentalExtension` 
and `getExperimentalExtensionVersion`) going out of sync if we have only one of 
them and make it return an `llvm::Optional` of the pair of versions.

Thent it can be used like this

```lang=cpp
if (auto ExperimentalExtension = isExperimentalExtension(Ext)) {
   std::pair SupportedVers = *ExperimentalVersion;
   ...
}
```

I'd also add a comment that the pair's `first` is the major version and 
`second` is the minor version (or alternative use a struct with two public 
fields `Major` and `Minor`)



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:396
+  I += Minor.size() + 1 /*'p'*/;
+if (*I == '_')
+  ++I;

There is no test for that case but I'm afraid we can't test it yet, can we?


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

https://reviews.llvm.org/D73891



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

[PATCH] D70799: [OpenMP] Lower taskyield using OpenMP IR Builder

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 updated this revision to Diff 233524.
rogfer01 added a comment.

ChangeLog:

- Rebase


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

https://reviews.llvm.org/D70799

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/taskyield_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  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
@@ -234,3 +234,20 @@
 
   return Builder.saveIP();
 }
+
+void OpenMPIRBuilder::emitTaskyieldImpl(const LocationDescription ) {
+  // Build call __kmpc_omp_taskyield(loc, thread_id, 0);
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocStr);
+  Constant *I32Null = ConstantInt::getNullValue(Int32);
+  Value *Args[] = {Ident, getOrCreateThreadID(Ident), I32Null};
+
+  Builder.CreateCall(getOrCreateRuntimeFunction(OMPRTL___kmpc_omp_taskyield),
+ Args);
+}
+
+void OpenMPIRBuilder::CreateTaskyield(const LocationDescription ) {
+  if (!updateToLocation(Loc))
+return;
+  emitTaskyieldImpl(Loc);
+}
Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -168,6 +168,8 @@
 __OMP_RTL(__kmpc_global_thread_num, false, Int32, IdentPtr)
 __OMP_RTL(__kmpc_fork_call, true, Void, IdentPtr, Int32, ParallelTaskPtr)
 __OMP_RTL(omp_get_thread_num, false, Int32, )
+__OMP_RTL(__kmpc_omp_taskwait, false, Int32, IdentPtr, Int32)
+__OMP_RTL(__kmpc_omp_taskyield, false, Int32, IdentPtr, Int32, Int32)
 
 #undef __OMP_RTL
 #undef OMP_RTL
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -73,6 +73,11 @@
   bool ForceSimpleCall = false,
   bool CheckCancelFlag = true);
 
+  /// Generator for '#omp taskyield'
+  ///
+  /// \param Loc The location where the taskyield directive was encountered.
+  void CreateTaskyield(const LocationDescription& Loc);
+
   ///}
 
 private:
@@ -112,6 +117,11 @@
 omp::Directive DK, bool ForceSimpleCall,
 bool CheckCancelFlag);
 
+  /// Generate a taskyield runtime call.
+  ///
+  /// \param Loc The location at which the request originated and is fulfilled.
+  void emitTaskyieldImpl(const LocationDescription );
+
   /// Return the current thread ID.
   ///
   /// \param Ident The ident (ident_t*) describing the query origin.
Index: clang/test/OpenMP/taskyield_codegen.cpp
===
--- clang/test/OpenMP/taskyield_codegen.cpp
+++ clang/test/OpenMP/taskyield_codegen.cpp
@@ -1,6 +1,10 @@
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+//
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -3249,11 +3249,18 @@
 SourceLocation Loc) {
   if (!CGF.HaveInsertPoint())
 return;
-  // Build call __kmpc_omp_taskyield(loc, thread_id, 0);
-  llvm::Value *Args[] = {
-  emitUpdateLocation(CGF, Loc), getThreadID(CGF, Loc),
-  llvm::ConstantInt::get(CGM.IntTy, /*V=*/0, /*isSigned=*/true)};
-  CGF.EmitRuntimeCall(createRuntimeFunction(OMPRTL__kmpc_omp_taskyield), Args);
+  llvm::OpenMPIRBuilder *OMPBuilder = CGF.CGM.getOpenMPIRBuilder();
+  if (OMPBuilder) {
+OMPBuilder->CreateTaskyield(CGF.Builder);
+  } else {
+// Build call __kmpc_omp_taskyield(loc, thread_id, 0);
+

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added inline comments.



Comment at: llvm/include/llvm/Frontend/OpenMPKinds.def:165
+
+__OMP_RTL(__kmpc_barrier, false, Void, IdentPtr, Int32)
+__OMP_RTL(__kmpc_cancel_barrier, false, Int32, IdentPtr, Int32)

jdoerfert wrote:
> rogfer01 wrote:
> > As we migrate, we will end with a significant number of interfaces here.
> > 
> > @jdoerfert what do you think about adding a comment with their C prototype 
> > before each one like we do in `clang/lib/CodeGen/CGOpenMPRuntime.cpp`?
> > 
> > Something like this
> > 
> > ```lang=cpp
> > // void __kmpc_barrier(ident_t *loc, kmp_int32 global_tid);
> > __OMP_RTL(__kmpc_barrier, false, Void, IdentPtr, Int32)
> > // kmp_int32 __kmpc_cancel_barrier(ident_t *loc, kmp_int32
> > // global_tid)
> > __OMP_RTL(__kmpc_cancel_barrier, false, Int32, IdentPtr, Int32)
> > ...
> > ```
> I'm fine with this but I doubt it'll help much (compared to the lines we have 
> that show name and types).
> 
> If you want this to happen you should create a patch do add comments for the 
> ones we have here, and others can way in. If there is agreement to apply it, 
> we will do so and continue that tradition from then on. Does that sound good?
Sounds reasonable to me. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D70799: [OpenMP] Lower taskyield using OpenMP IR Builder

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 created this revision.
rogfer01 added reviewers: jdoerfert, kiranchandramohan.
Herald added subscribers: llvm-commits, cfe-commits, guansong, hiraditya.
Herald added projects: clang, LLVM.
rogfer01 added a parent revision: D69922: [OpenMP] Use the OpenMP-IR-Builder.

This is similar to D69828 .

Special codegen for enclosing untied tasks is still done in clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70799

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/taskyield_codegen.cpp
  llvm/include/llvm/Frontend/OpenMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMPKinds.def
  llvm/lib/Frontend/OpenMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMPIRBuilder.cpp
@@ -272,3 +272,20 @@
 return;
   emitTaskwaitImpl(Loc);
 }
+
+void OpenMPIRBuilder::emitTaskyieldImpl(const LocationDescription ) {
+  // Build call __kmpc_omp_taskyield(loc, thread_id, 0);
+  Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+  Value *Ident = getOrCreateIdent(SrcLocStr);
+  Constant *I32Null = ConstantInt::getNullValue(Int32);
+  Value *Args[] = {Ident, getOrCreateThreadID(Ident), I32Null};
+
+  Builder.CreateCall(getOrCreateRuntimeFunction(OMPRTL___kmpc_omp_taskyield),
+ Args);
+}
+
+void OpenMPIRBuilder::CreateTaskyield(const LocationDescription ) {
+  if (!updateToLocation(Loc))
+return;
+  emitTaskyieldImpl(Loc);
+}
Index: llvm/include/llvm/Frontend/OpenMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMPKinds.def
@@ -168,6 +168,7 @@
 __OMP_RTL(__kmpc_fork_call, true, Void, IdentPtr, Int32, ParallelTaskPtr)
 __OMP_RTL(omp_get_thread_num, false, Int32, )
 __OMP_RTL(__kmpc_omp_taskwait, false, Int32, IdentPtr, Int32)
+__OMP_RTL(__kmpc_omp_taskyield, false, Int32, IdentPtr, Int32, Int32)
 
 #undef __OMP_RTL
 #undef OMP_RTL
Index: llvm/include/llvm/Frontend/OpenMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMPIRBuilder.h
@@ -78,6 +78,11 @@
   /// \param Loc The location where the taskwait directive was encountered.
   void CreateTaskwait(const LocationDescription& Loc);
 
+  /// Generator for '#omp taskyield'
+  ///
+  /// \param Loc The location where the taskyield directive was encountered.
+  void CreateTaskyield(const LocationDescription& Loc);
+
   ///}
 
 private:
@@ -122,6 +127,11 @@
   /// \param Loc The location at which the request originated and is fulfilled.
   void emitTaskwaitImpl(const LocationDescription );
 
+  /// Generate a taskyield runtime call.
+  ///
+  /// \param Loc The location at which the request originated and is fulfilled.
+  void emitTaskyieldImpl(const LocationDescription );
+
   /// Return the current thread ID.
   ///
   /// \param Ident The ident (ident_t*) describing the query origin.
Index: clang/test/OpenMP/taskyield_codegen.cpp
===
--- clang/test/OpenMP/taskyield_codegen.cpp
+++ clang/test/OpenMP/taskyield_codegen.cpp
@@ -1,6 +1,10 @@
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+//
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-unknown-unknown -std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -x c++ -triple x86_64-unknown-unknown -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++11 -triple x86_64-unknown-unknown -emit-pch -o %t %s
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -3247,11 +3247,18 @@
 SourceLocation Loc) {
   if (!CGF.HaveInsertPoint())
 return;
-  // Build call __kmpc_omp_taskyield(loc, thread_id, 0);
-  llvm::Value *Args[] = {
-  emitUpdateLocation(CGF, Loc), getThreadID(CGF, Loc),
-  llvm::ConstantInt::get(CGM.IntTy, /*V=*/0, /*isSigned=*/true)};
-  CGF.EmitRuntimeCall(createRuntimeFunction(OMPRTL__kmpc_omp_taskyield), Args);
+  

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added inline comments.



Comment at: llvm/include/llvm/Frontend/OpenMPKinds.def:165
+
+__OMP_RTL(__kmpc_barrier, false, Void, IdentPtr, Int32)
+__OMP_RTL(__kmpc_cancel_barrier, false, Int32, IdentPtr, Int32)

As we migrate, we will end with a significant number of interfaces here.

@jdoerfert what do you think about adding a comment with their C prototype 
before each one like we do in `clang/lib/CodeGen/CGOpenMPRuntime.cpp`?

Something like this

```lang=cpp
// void __kmpc_barrier(ident_t *loc, kmp_int32 global_tid);
__OMP_RTL(__kmpc_barrier, false, Void, IdentPtr, Int32)
// kmp_int32 __kmpc_cancel_barrier(ident_t *loc, kmp_int32
// global_tid)
__OMP_RTL(__kmpc_cancel_barrier, false, Int32, IdentPtr, Int32)
...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

In D69785#1733233 , @jdoerfert wrote:

> In D69785#1732951 , @rogfer01 wrote:
>
> > I made a small experiment lowering a `taskwait` (which is even simpler than 
> > `barrier` in "lowering" complexity). It was pretty straightforward after 
> > all.
>
>
> Nice, can you share the code? Was it based on the OpenMPIRBuilder (this one 
> or the old one) or somehow separate?


I posted the WIP D69828 .

Kind regards,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Hi @jdoerfert, thanks a lot for putting this up this initial skeleton and 
providing an example with `barrier`.

I made a small experiment lowering a `taskwait` (which is even simpler than 
`barrier` in "lowering" complexity). It was pretty straightforward after all.

I have a few questions just to reassure I fully understand your vision here:

- the parameter `Loc` is currently unused but my understanding from the comment 
in `OpenMPIRBuilder::getSrcLocStr` eventually we will convert a 
`clang::SourceLocation` to a `llvm::Constant*` that represents the location as 
used by KMP, right?
- emitting a `taskwait` however has this

  if (auto *Region = dyn_cast_or_null(CGF.CapturedStmtInfo))
Region->emitUntiedSwitch(CGF);

but my understanding is that we can apply the same scheme to these hooks as 
well, recursively, i.e.: progressively convert them to calls to 
`OpenMPIRBuilder` (if needed under the flag suggested above by Alexey). Does 
this make sense?

Kind regards,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D69383: [RISCV] Match GCC `-march`/`-mabi` driver defaults

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:475
+
+if (MArch.startswith_lower("rv32")) {
+  if (MArch.substr(4).contains_lower("d") ||

lenary wrote:
> rogfer01 wrote:
> > `llvm::StringSwitch` has a method `StartsWithLower` which might help make 
> > the logic a bit clearer
> > 
> > Something like this (I haven't tested it!)
> > 
> > ```lang=cpp
> > StringRef ABIFromMarch = StringSwitch(MArch)
> >.StartsWithLower("rv32d", "ilp32d")
> >.StartsWithLower("rv32g", "ilp32d")
> >.StartsWithLower("rv32e", "ilp32e")
> >.StartsWithLower("rv32", "ilp32")
> > 
> >.StartsWithLower("rv64d", "lp64d")
> >.StartsWithLower("rv64g", "lp64d")
> >.StartsWithLower("rv64", "lp64").
> > 
> >.Default("");
> > 
> > if (!ABIFromMarch.empty()) return ABIFromMarch;
> > ```
> Sadly I don't think this will work, because of the case of matching `rv32*d*` 
> and `rv64*d*` (the `March.substr(4).contains_lower("d")` cases) from 
> config.gcc. Explicitly "d" does not come immediately after `rv<32/64>`, it 
> can come anywhere after like in `rv32imafdc`.
> 
> The other issue I have with the StringSwitch is that it requires I have a 
> default, which I feel makes the control flow harder to understand, rather 
> than easier. 
Oh I see.

Then I would comment what this part does with a bit more detail right after the 
`// 2. Choose a default based on -march=`. For example

```
// rv32g | rv32*d -> ilp32d
// rv32e -> ilp32e
// rv32* -> ilp32
// rv64g | rv64*d -> lp64d
// rv64* -> lp64
```

Given that gcc is using a shell glob, then `GlobPattern` in 
`Support/GlobPattern.h` may help as well. But it might be overkill in this 
scenario.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69383



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

[PATCH] D69383: [RISCV] Match GCC `-march`/`-mabi` driver defaults

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:475
+
+if (MArch.startswith_lower("rv32")) {
+  if (MArch.substr(4).contains_lower("d") ||

`llvm::StringSwitch` has a method `StartsWithLower` which might help make the 
logic a bit clearer

Something like this (I haven't tested it!)

```lang=cpp
StringRef ABIFromMarch = StringSwitch(MArch)
   .StartsWithLower("rv32d", "ilp32d")
   .StartsWithLower("rv32g", "ilp32d")
   .StartsWithLower("rv32e", "ilp32e")
   .StartsWithLower("rv32", "ilp32")

   .StartsWithLower("rv64d", "lp64d")
   .StartsWithLower("rv64g", "lp64d")
   .StartsWithLower("rv64", "lp64").

   .Default("");

if (!ABIFromMarch.empty()) return ABIFromMarch;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69383



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

[PATCH] D20561: Warn when taking address of packed member

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf7b9f3149b76: Add missing tests (authored by rogfer01).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D20561?vs=67807=223564#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D20561

Files:
  clang/test/Sema/address-packed-member-memops.c
  clang/test/Sema/address-packed.c
  clang/test/SemaCXX/address-packed-member-memops.cpp
  clang/test/SemaCXX/address-packed.cpp

Index: clang/test/SemaCXX/address-packed.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/address-packed.cpp
@@ -0,0 +1,114 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+extern void f1(int *);
+extern void f2(char *);
+
+struct __attribute__((packed)) Arguable {
+  int x;
+  char c;
+  static void foo();
+};
+
+extern void f3(void());
+
+namespace Foo {
+struct __attribute__((packed)) Arguable {
+  char c;
+  int x;
+  static void foo();
+};
+}
+
+struct Arguable *get_arguable();
+
+void f4(int &);
+
+void to_void(void *);
+
+template 
+void sink(T...);
+
+void g0() {
+  {
+Foo::Arguable arguable;
+f1();   // expected-warning {{packed member 'x' of class or structure 'Foo::Arguable'}}
+f2();   // no-warning
+f3(); // no-warning
+
+to_void(); // no-warning
+void *p1 =// no-warning
+void *p2 = static_cast();  // no-warning
+void *p3 = reinterpret_cast(); // no-warning
+void *p4 = (void *)   // no-warning
+sink(p1, p2, p3, p4);
+  }
+  {
+Arguable arguable1;
+Arguable (arguable1);
+f1();   // expected-warning {{packed member 'x' of class or structure 'Arguable'}}
+f2();   // no-warning
+f3(); // no-warning
+  }
+  {
+Arguable *arguable1;
+Arguable *(arguable1);
+f1(>x);   // expected-warning {{packed member 'x' of class or structure 'Arguable'}}
+f2(>c);   // no-warning
+f3(>foo); // no-warning
+  }
+}
+
+struct __attribute__((packed)) A {
+  int x;
+  char c;
+
+  int *f0() {
+return >x; // expected-warning {{packed member 'x' of class or structure 'A'}}
+  }
+
+  int *g0() {
+return  // expected-warning {{packed member 'x' of class or structure 'A'}}
+  }
+
+  char *h0() {
+return  // no-warning
+  }
+};
+
+struct B : A {
+  int *f1() {
+return >x; // expected-warning {{packed member 'x' of class or structure 'A'}}
+  }
+
+  int *g1() {
+return  // expected-warning {{packed member 'x' of class or structure 'A'}}
+  }
+
+  char *h1() {
+return  // no-warning
+  }
+};
+
+template 
+class __attribute__((packed)) S {
+  Ty X;
+
+public:
+  const Ty *get() const {
+return  // expected-warning {{packed member 'X' of class or structure 'S'}}
+   // expected-warning@-1 {{packed member 'X' of class or structure 'S'}}
+  }
+};
+
+template 
+void h(Ty *);
+
+void g1() {
+  S s1;
+  s1.get(); // expected-note {{in instantiation of member function 'S::get'}}
+
+  S s2;
+  s2.get();
+
+  S s3;
+  s3.get(); // expected-note {{in instantiation of member function 'S::get'}}
+}
Index: clang/test/SemaCXX/address-packed-member-memops.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/address-packed-member-memops.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+struct B {
+  int x, y, z, w;
+} b;
+
+struct __attribute__((packed)) A {
+  struct B b;
+} a;
+
+typedef __typeof__(sizeof(int)) size_t;
+
+extern "C" {
+void *memcpy(void *dest, const void *src, size_t n);
+int memcmp(const void *s1, const void *s2, size_t n);
+void *memmove(void *dest, const void *src, size_t n);
+void *memset(void *s, int c, size_t n);
+}
+
+int x;
+
+void foo() {
+  memcpy(, , sizeof(b));
+  memmove(, , sizeof(b));
+  memset(, 0, sizeof(b));
+  x = memcmp(, , sizeof(b));
+}
Index: clang/test/Sema/address-packed.c
===
--- /dev/null
+++ clang/test/Sema/address-packed.c
@@ -0,0 +1,163 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#include 
+
+extern void f1(int *);
+extern void f2(char *);
+
+struct Ok {
+  char c;
+  int x;
+};
+
+struct __attribute__((packed)) Arguable {
+  char c0;
+  int x;
+  char c1;
+};
+
+union __attribute__((packed)) UnionArguable {
+  char c;
+  int x;
+};
+
+typedef struct Arguable ArguableT;
+
+struct Arguable *get_arguable();
+
+void to_void(void *);
+
+void g0(void) {
+  {
+struct Ok ok;
+f1(); // no-warning
+f2(); // no-warning
+  }
+  {
+struct Arguable arguable;
+f2(); // no-warning
+f1();  // expected-warning {{packed member 'x' of class or structure 'Arguable'}}
+f2(); // no-warning
+
+f1((int *)(void *)); // no-warning
+to_void();   // no-warning
+void *p =  

[PATCH] D66003: [RISCV] Make -march=rv{32, 64}gc the default in RISC-V Linux

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371496: [RISCV] Make -march=rv{32,64}gc the default in 
RISC-V Linux (authored by rogfer01, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66003?vs=214345=219488#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66003

Files:
  cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
  cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.h
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/test/Driver/riscv-features.c


Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.h
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.h
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.h
@@ -19,7 +19,8 @@
 namespace driver {
 namespace tools {
 namespace riscv {
-void getRISCVTargetFeatures(const Driver , const llvm::opt::ArgList ,
+void getRISCVTargetFeatures(const Driver , const llvm::Triple ,
+const llvm::opt::ArgList ,
 std::vector );
 StringRef getRISCVABI(const llvm::opt::ArgList ,
   const llvm::Triple );
Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -12,6 +12,7 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/raw_ostream.h"
 #include "ToolChains/CommonArgs.h"
@@ -353,14 +354,18 @@
   return true;
 }
 
-void riscv::getRISCVTargetFeatures(const Driver , const ArgList ,
+void riscv::getRISCVTargetFeatures(const Driver , const llvm::Triple ,
+   const ArgList ,
std::vector ) {
-  if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
-StringRef MArch = A->getValue();
+  llvm::Optional MArch;
+  if (const Arg *A = Args.getLastArg(options::OPT_march_EQ))
+MArch = A->getValue();
+  else if (Triple.getOS() == llvm::Triple::Linux)
+// RISC-V Linux defaults to rv{32,64}gc.
+MArch = Triple.getArch() == llvm::Triple::riscv32 ? "rv32gc" : "rv64gc";
 
-if (!getArchFeatures(D, MArch, Features, Args))
-  return;
-  }
+  if (MArch.hasValue() && !getArchFeatures(D, *MArch, Features, Args))
+return;
 
   // -mrelax is default, unless -mno-relax is specified.
   if (Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, true))
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -336,7 +336,7 @@
 break;
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
-riscv::getRISCVTargetFeatures(D, Args, Features);
+riscv::getRISCVTargetFeatures(D, Triple, Args, Features);
 break;
   case llvm::Triple::systemz:
 systemz::getSystemZTargetFeatures(Args, Features);
Index: cfe/trunk/test/Driver/riscv-features.c
===
--- cfe/trunk/test/Driver/riscv-features.c
+++ cfe/trunk/test/Driver/riscv-features.c
@@ -18,4 +18,15 @@
 
 // SAVE-RESTORE: warning: the clang compiler does not support '-msave-restore'
 // NO-SAVE-RESTORE-NOT: warning: the clang compiler does not support
-// DEFAULT-NOT: warning: the clang compiler does not support
\ No newline at end of file
+// DEFAULT-NOT: warning: the clang compiler does not support
+
+// RUN: %clang -target riscv32-linux -### %s -fsyntax-only 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=DEFAULT-LINUX
+// RUN: %clang -target riscv64-linux -### %s -fsyntax-only 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=DEFAULT-LINUX
+
+// DEFAULT-LINUX: "-target-feature" "+m"
+// DEFAULT-LINUX-SAME: "-target-feature" "+a"
+// DEFAULT-LINUX-SAME: "-target-feature" "+f"
+// DEFAULT-LINUX-SAME: "-target-feature" "+d"
+// DEFAULT-LINUX-SAME: "-target-feature" "+c"


Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.h
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.h
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.h
@@ -19,7 +19,8 @@
 namespace driver {
 namespace tools {
 namespace riscv {
-void getRISCVTargetFeatures(const Driver , const llvm::opt::ArgList ,
+void getRISCVTargetFeatures(const Driver , const llvm::Triple ,
+const llvm::opt::ArgList ,
 std::vector );
 StringRef getRISCVABI(const llvm::opt::ArgList ,
   const llvm::Triple );
Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp

[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371494: [RISCV] Default to ilp32d/lp64d in RISC-V Linux 
(authored by rogfer01, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65634?vs=213812=219487#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65634

Files:
  cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
  cfe/trunk/test/Driver/riscv32-toolchain.c
  cfe/trunk/test/Driver/riscv64-toolchain.c
  cfe/trunk/test/Preprocessor/riscv-target-features.c


Index: cfe/trunk/test/Driver/riscv32-toolchain.c
===
--- cfe/trunk/test/Driver/riscv32-toolchain.c
+++ cfe/trunk/test/Driver/riscv32-toolchain.c
@@ -68,7 +68,7 @@
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv32-unknown-linux-gnu \
+// RUN:   -target riscv32-unknown-linux-gnu -mabi=ilp32 \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-LINUX-MULTI-ILP32 %s
@@ -84,7 +84,7 @@
 // C-RV32-LINUX-MULTI-ILP32: 
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv32-unknown-linux-gnu -march=rv32imafd -mabi=ilp32d \
+// RUN:   -target riscv32-unknown-linux-gnu -march=rv32imafd \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-LINUX-MULTI-ILP32D %s
Index: cfe/trunk/test/Driver/riscv64-toolchain.c
===
--- cfe/trunk/test/Driver/riscv64-toolchain.c
+++ cfe/trunk/test/Driver/riscv64-toolchain.c
@@ -68,7 +68,7 @@
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv64-unknown-linux-gnu \
+// RUN:   -target riscv64-unknown-linux-gnu -mabi=lp64 \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-LINUX-MULTI-LP64 %s
@@ -84,7 +84,7 @@
 // C-RV64-LINUX-MULTI-LP64: 
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv64-unknown-linux-gnu -march=rv64imafd -mabi=lp64d \
+// RUN:   -target riscv64-unknown-linux-gnu -march=rv64imafd \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-LINUX-MULTI-LP64D %s
Index: cfe/trunk/test/Preprocessor/riscv-target-features.c
===
--- cfe/trunk/test/Preprocessor/riscv-target-features.c
+++ cfe/trunk/test/Preprocessor/riscv-target-features.c
@@ -48,9 +48,9 @@
 // RUN: -o - | FileCheck --check-prefix=CHECK-C-EXT %s
 // CHECK-C-EXT: __riscv_compressed 1
 
-// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -x c -E -dM %s 
\
+// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -mabi=ilp32 -x 
c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SOFT %s
-// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -x c -E -dM %s 
\
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -mabi=lp64 -x 
c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SOFT %s
 // CHECK-SOFT: __riscv_float_abi_soft 1
 // CHECK-SOFT-NOT: __riscv_float_abi_single
@@ -64,9 +64,9 @@
 // CHECK-SINGLE-NOT: __riscv_float_abi_soft
 // CHECK-SINGLE-NOT: __riscv_float_abi_double
 
-// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -mabi=ilp32d 
-x c -E -dM %s \
+// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -x c -E -dM %s 
\
 // RUN: -o - | FileCheck --check-prefix=CHECK-DOUBLE %s
-// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -mabi=lp64d -x 
c -E -dM %s \
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -x c -E -dM %s 
\
 // RUN: -o - | FileCheck --check-prefix=CHECK-DOUBLE %s
 // CHECK-DOUBLE: __riscv_float_abi_double 1
 // CHECK-DOUBLE-NOT: __riscv_float_abi_soft
Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -389,7 +389,9 @@
   if (const Arg *A = 

[PATCH] D66003: [RISCV] Make -march=rv{32, 64}gc the default in RISC-V Linux

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.
Herald added a subscriber: pzheng.

Thanks for the review @luismarques

I plan to commit this shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66003



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

[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Thanks for the review @lenary @luismarques

We can indeed look at what defaults we want for baremetal in a later change.

I plan to commit this shortly.


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

https://reviews.llvm.org/D65634



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

[PATCH] D66002: [RISCV] Move architecture parsing code into its own function

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL371492: [RISCV] Move architecture parsing code into its own 
function (authored by rogfer01, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66002?vs=214344=219482#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66002

Files:
  cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp

Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -189,167 +189,177 @@
   }
 }
 
-void riscv::getRISCVTargetFeatures(const Driver , const ArgList ,
-   std::vector ) {
-  if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
-StringRef MArch = A->getValue();
+// Returns false if an error is diagnosed.
+static bool getArchFeatures(const Driver , StringRef MArch,
+std::vector ,
+const ArgList ) {
+  // RISC-V ISA strings must be lowercase.
+  if (llvm::any_of(MArch, [](char c) { return isupper(c); })) {
+D.Diag(diag::err_drv_invalid_riscv_arch_name)
+<< MArch << "string must be lowercase";
+return false;
+  }
 
-// RISC-V ISA strings must be lowercase.
-if (llvm::any_of(MArch, [](char c) { return isupper(c); })) {
-  D.Diag(diag::err_drv_invalid_riscv_arch_name)
-  << MArch << "string must be lowercase";
-  return;
-}
+  // ISA string must begin with rv32 or rv64.
+  if (!(MArch.startswith("rv32") || MArch.startswith("rv64")) ||
+  (MArch.size() < 5)) {
+D.Diag(diag::err_drv_invalid_riscv_arch_name)
+<< MArch << "string must begin with rv32{i,e,g} or rv64{i,g}";
+return false;
+  }
 
-// ISA string must begin with rv32 or rv64.
-if (!(MArch.startswith("rv32") || MArch.startswith("rv64")) ||
-(MArch.size() < 5)) {
-  D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch
-<< "string must begin with rv32{i,e,g} or rv64{i,g}";
-  return;
-}
+  bool HasRV64 = MArch.startswith("rv64");
 
-bool HasRV64 = MArch.startswith("rv64");
+  // The canonical order specified in ISA manual.
+  // Ref: Table 22.1 in RISC-V User-Level ISA V2.2
+  StringRef StdExts = "mafdqlcbjtpvn";
+  bool HasF = false, HasD = false;
+  char Baseline = MArch[4];
+
+  // First letter should be 'e', 'i' or 'g'.
+  switch (Baseline) {
+  default:
+D.Diag(diag::err_drv_invalid_riscv_arch_name)
+<< MArch << "first letter should be 'e', 'i' or 'g'";
+return false;
+  case 'e': {
+StringRef Error;
+// Currently LLVM does not support 'e'.
+// Extension 'e' is not allowed in rv64.
+if (HasRV64)
+  Error = "standard user-level extension 'e' requires 'rv32'";
+else
+  Error = "unsupported standard user-level extension 'e'";
+D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch << Error;
+return false;
+  }
+  case 'i':
+break;
+  case 'g':
+// g = imafd
+StdExts = StdExts.drop_front(4);
+Features.push_back("+m");
+Features.push_back("+a");
+Features.push_back("+f");
+Features.push_back("+d");
+HasF = true;
+HasD = true;
+break;
+  }
 
-// The canonical order specified in ISA manual.
-// Ref: Table 22.1 in RISC-V User-Level ISA V2.2
-StringRef StdExts = "mafdqlcbjtpvn";
-bool HasF = false, HasD = false;
-char Baseline = MArch[4];
+  // Skip rvxxx
+  StringRef Exts = MArch.substr(5);
 
-// First letter should be 'e', 'i' or 'g'.
-switch (Baseline) {
-default:
-  D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch
-<< "first letter should be 'e', 'i' or 'g'";
-  return;
-case 'e': {
+  // Remove non-standard extensions and supervisor-level extensions.
+  // They have 'x', 's', 'sx' prefixes. Parse them at the end.
+  // Find the very first occurrence of 's' or 'x'.
+  StringRef OtherExts;
+  size_t Pos = Exts.find_first_of("sx");
+  if (Pos != StringRef::npos) {
+OtherExts = Exts.substr(Pos);
+Exts = Exts.substr(0, Pos);
+  }
+
+  std::string Major, Minor;
+  if (!getExtensionVersion(D, MArch, std::string(1, Baseline), Exts, Major,
+   Minor))
+return false;
+
+  // TODO: Use version number when setting target features
+  // and consume the underscore '_' that might follow.
+
+  auto StdExtsItr = StdExts.begin();
+  auto StdExtsEnd = StdExts.end();
+
+  for (auto I = Exts.begin(), E = Exts.end(); I != E; ++I) {
+char c = *I;
+
+// Check ISA extensions are specified in the canonical order.
+while (StdExtsItr != StdExtsEnd && *StdExtsItr != c)
+  ++StdExtsItr;
+
+if (StdExtsItr == StdExtsEnd) {
+  // Either c contains a valid extension but it was not given 

[PATCH] D66002: [RISCV] Move architecture parsing code into its own function

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.
Herald added a subscriber: pzheng.

Thanks for the review @luismarques

I plan to commit this shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66002



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

[PATCH] D66003: [RISCV] Make -march=rv{32, 64}gc the default in RISC-V Linux

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 created this revision.
rogfer01 added reviewers: asb, lenary.
Herald added subscribers: cfe-commits, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, MaskRay, 
jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, apazos, simoncook, johnrusso, 
rbar.
Herald added a project: clang.
rogfer01 added parent revisions: D66002: [RISCV] Move architecture parsing code 
into its own function, D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux.

This is the logical follow-up of D65634 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66003

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/riscv-features.c


Index: clang/test/Driver/riscv-features.c
===
--- clang/test/Driver/riscv-features.c
+++ clang/test/Driver/riscv-features.c
@@ -18,4 +18,15 @@
 
 // SAVE-RESTORE: warning: the clang compiler does not support '-msave-restore'
 // NO-SAVE-RESTORE-NOT: warning: the clang compiler does not support
-// DEFAULT-NOT: warning: the clang compiler does not support
\ No newline at end of file
+// DEFAULT-NOT: warning: the clang compiler does not support
+
+// RUN: %clang -target riscv32-linux -### %s -fsyntax-only 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=DEFAULT-LINUX
+// RUN: %clang -target riscv64-linux -### %s -fsyntax-only 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=DEFAULT-LINUX
+
+// DEFAULT-LINUX: "-target-feature" "+m"
+// DEFAULT-LINUX-SAME: "-target-feature" "+a"
+// DEFAULT-LINUX-SAME: "-target-feature" "+f"
+// DEFAULT-LINUX-SAME: "-target-feature" "+d"
+// DEFAULT-LINUX-SAME: "-target-feature" "+c"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -336,7 +336,7 @@
 break;
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
-riscv::getRISCVTargetFeatures(D, Args, Features);
+riscv::getRISCVTargetFeatures(D, Triple, Args, Features);
 break;
   case llvm::Triple::systemz:
 systemz::getSystemZTargetFeatures(Args, Features);
Index: clang/lib/Driver/ToolChains/Arch/RISCV.h
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.h
+++ clang/lib/Driver/ToolChains/Arch/RISCV.h
@@ -19,7 +19,8 @@
 namespace driver {
 namespace tools {
 namespace riscv {
-void getRISCVTargetFeatures(const Driver , const llvm::opt::ArgList ,
+void getRISCVTargetFeatures(const Driver , const llvm::Triple ,
+const llvm::opt::ArgList ,
 std::vector );
 StringRef getRISCVABI(const llvm::opt::ArgList ,
   const llvm::Triple );
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -12,6 +12,7 @@
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/TargetParser.h"
 #include "llvm/Support/raw_ostream.h"
 #include "ToolChains/CommonArgs.h"
@@ -353,14 +354,18 @@
   return true;
 }
 
-void riscv::getRISCVTargetFeatures(const Driver , const ArgList ,
+void riscv::getRISCVTargetFeatures(const Driver , const llvm::Triple ,
+   const ArgList ,
std::vector ) {
-  if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
-StringRef MArch = A->getValue();
-
-if (!getArchFeatures(D, MArch, Features, Args))
-  return;
-  }
+  llvm::Optional MArch;
+  if (const Arg *A = Args.getLastArg(options::OPT_march_EQ))
+MArch = A->getValue();
+  else if (Triple.getOS() == llvm::Triple::Linux)
+// RISC-V Linux defaults to rv{32,64}gc.
+MArch = Triple.getArch() == llvm::Triple::riscv32 ? "rv32gc" : "rv64gc";
+
+  if (MArch.hasValue() && !getArchFeatures(D, *MArch, Features, Args))
+return;
 
   // -mrelax is default, unless -mno-relax is specified.
   if (Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, true))


Index: clang/test/Driver/riscv-features.c
===
--- clang/test/Driver/riscv-features.c
+++ clang/test/Driver/riscv-features.c
@@ -18,4 +18,15 @@
 
 // SAVE-RESTORE: warning: the clang compiler does not support '-msave-restore'
 // NO-SAVE-RESTORE-NOT: warning: the clang compiler does not support
-// DEFAULT-NOT: warning: the clang compiler does not support
\ No newline at end of file
+// DEFAULT-NOT: warning: the clang compiler does not support
+
+// RUN: %clang -target riscv32-linux -### %s -fsyntax-only 2>&1 \
+// RUN:   | 

[PATCH] D66002: [RISCV] Move architecture parsing code into its own function

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 created this revision.
rogfer01 added reviewers: asb, lenary.
Herald added subscribers: cfe-commits, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, MaskRay, 
jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, apazos, simoncook, johnrusso, 
rbar.
Herald added a project: clang.

I plan to reuse it in a later patch.

This is almost NFC except a small change in control flow when diagnosing `+d` 
without `+f`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66002

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp

Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -189,167 +189,177 @@
   }
 }
 
-void riscv::getRISCVTargetFeatures(const Driver , const ArgList ,
-   std::vector ) {
-  if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
-StringRef MArch = A->getValue();
+// Returns false if an error is diagnosed.
+static bool getArchFeatures(const Driver , StringRef MArch,
+std::vector ,
+const ArgList ) {
+  // RISC-V ISA strings must be lowercase.
+  if (llvm::any_of(MArch, [](char c) { return isupper(c); })) {
+D.Diag(diag::err_drv_invalid_riscv_arch_name)
+<< MArch << "string must be lowercase";
+return false;
+  }
 
-// RISC-V ISA strings must be lowercase.
-if (llvm::any_of(MArch, [](char c) { return isupper(c); })) {
-  D.Diag(diag::err_drv_invalid_riscv_arch_name)
-  << MArch << "string must be lowercase";
-  return;
-}
+  // ISA string must begin with rv32 or rv64.
+  if (!(MArch.startswith("rv32") || MArch.startswith("rv64")) ||
+  (MArch.size() < 5)) {
+D.Diag(diag::err_drv_invalid_riscv_arch_name)
+<< MArch << "string must begin with rv32{i,e,g} or rv64{i,g}";
+return false;
+  }
 
-// ISA string must begin with rv32 or rv64.
-if (!(MArch.startswith("rv32") || MArch.startswith("rv64")) ||
-(MArch.size() < 5)) {
-  D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch
-<< "string must begin with rv32{i,e,g} or rv64{i,g}";
-  return;
-}
+  bool HasRV64 = MArch.startswith("rv64");
+
+  // The canonical order specified in ISA manual.
+  // Ref: Table 22.1 in RISC-V User-Level ISA V2.2
+  StringRef StdExts = "mafdqlcbjtpvn";
+  bool HasF = false, HasD = false;
+  char Baseline = MArch[4];
+
+  // First letter should be 'e', 'i' or 'g'.
+  switch (Baseline) {
+  default:
+D.Diag(diag::err_drv_invalid_riscv_arch_name)
+<< MArch << "first letter should be 'e', 'i' or 'g'";
+return false;
+  case 'e': {
+StringRef Error;
+// Currently LLVM does not support 'e'.
+// Extension 'e' is not allowed in rv64.
+if (HasRV64)
+  Error = "standard user-level extension 'e' requires 'rv32'";
+else
+  Error = "unsupported standard user-level extension 'e'";
+D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch << Error;
+return false;
+  }
+  case 'i':
+break;
+  case 'g':
+// g = imafd
+StdExts = StdExts.drop_front(4);
+Features.push_back("+m");
+Features.push_back("+a");
+Features.push_back("+f");
+Features.push_back("+d");
+HasF = true;
+HasD = true;
+break;
+  }
 
-bool HasRV64 = MArch.startswith("rv64");
+  // Skip rvxxx
+  StringRef Exts = MArch.substr(5);
+
+  // Remove non-standard extensions and supervisor-level extensions.
+  // They have 'x', 's', 'sx' prefixes. Parse them at the end.
+  // Find the very first occurrence of 's' or 'x'.
+  StringRef OtherExts;
+  size_t Pos = Exts.find_first_of("sx");
+  if (Pos != StringRef::npos) {
+OtherExts = Exts.substr(Pos);
+Exts = Exts.substr(0, Pos);
+  }
 
-// The canonical order specified in ISA manual.
-// Ref: Table 22.1 in RISC-V User-Level ISA V2.2
-StringRef StdExts = "mafdqlcbjtpvn";
-bool HasF = false, HasD = false;
-char Baseline = MArch[4];
+  std::string Major, Minor;
+  if (!getExtensionVersion(D, MArch, std::string(1, Baseline), Exts, Major,
+   Minor))
+return false;
 
-// First letter should be 'e', 'i' or 'g'.
-switch (Baseline) {
-default:
-  D.Diag(diag::err_drv_invalid_riscv_arch_name) << MArch
-<< "first letter should be 'e', 'i' or 'g'";
-  return;
-case 'e': {
+  // TODO: Use version number when setting target features
+  // and consume the underscore '_' that might follow.
+
+  auto StdExtsItr = StdExts.begin();
+  auto StdExtsEnd = StdExts.end();
+
+  for (auto I = Exts.begin(), E = Exts.end(); I != E; ++I) {
+char c = *I;
+
+// Check ISA extensions are specified in the canonical order.
+while (StdExtsItr != StdExtsEnd && *StdExtsItr != c)
+  ++StdExtsItr;
+
+if (StdExtsItr == StdExtsEnd) {
+

[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Thanks for the clarification @asb. I've posted D66003 
 (depending on D66002 
) for that.


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

https://reviews.llvm.org/D65634



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

[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 updated this revision to Diff 213812.
rogfer01 retitled this revision from "[RISCV] Default to lp64d in 64-bit RISC-V 
Linux" to "[RISCV] Default to ilp32d/lp64d in RISC-V Linux".
rogfer01 edited the summary of this revision.
rogfer01 added a comment.

ChangeLog:

- Make `ilp32d` also the default in 32-bit RISC-V Linux
- Do not use nested conditional expressions


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

https://reviews.llvm.org/D65634

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv32-toolchain.c
  clang/test/Driver/riscv64-toolchain.c
  clang/test/Preprocessor/riscv-target-features.c


Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -48,9 +48,9 @@
 // RUN: -o - | FileCheck --check-prefix=CHECK-C-EXT %s
 // CHECK-C-EXT: __riscv_compressed 1
 
-// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -x c -E -dM %s 
\
+// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -mabi=ilp32 -x 
c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SOFT %s
-// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -x c -E -dM %s 
\
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -mabi=lp64 -x 
c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SOFT %s
 // CHECK-SOFT: __riscv_float_abi_soft 1
 // CHECK-SOFT-NOT: __riscv_float_abi_single
@@ -64,9 +64,9 @@
 // CHECK-SINGLE-NOT: __riscv_float_abi_soft
 // CHECK-SINGLE-NOT: __riscv_float_abi_double
 
-// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -mabi=ilp32d 
-x c -E -dM %s \
+// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -x c -E -dM %s 
\
 // RUN: -o - | FileCheck --check-prefix=CHECK-DOUBLE %s
-// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -mabi=lp64d -x 
c -E -dM %s \
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -x c -E -dM %s 
\
 // RUN: -o - | FileCheck --check-prefix=CHECK-DOUBLE %s
 // CHECK-DOUBLE: __riscv_float_abi_double 1
 // CHECK-DOUBLE-NOT: __riscv_float_abi_soft
Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -68,7 +68,7 @@
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv64-unknown-linux-gnu \
+// RUN:   -target riscv64-unknown-linux-gnu -mabi=lp64 \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-LINUX-MULTI-LP64 %s
@@ -84,7 +84,7 @@
 // C-RV64-LINUX-MULTI-LP64: 
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv64-unknown-linux-gnu -march=rv64imafd -mabi=lp64d \
+// RUN:   -target riscv64-unknown-linux-gnu -march=rv64imafd \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-LINUX-MULTI-LP64D %s
Index: clang/test/Driver/riscv32-toolchain.c
===
--- clang/test/Driver/riscv32-toolchain.c
+++ clang/test/Driver/riscv32-toolchain.c
@@ -68,7 +68,7 @@
 // CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: 
"{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv32-unknown-linux-gnu \
+// RUN:   -target riscv32-unknown-linux-gnu -mabi=ilp32 \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-LINUX-MULTI-ILP32 %s
@@ -84,7 +84,7 @@
 // C-RV32-LINUX-MULTI-ILP32: 
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv32-unknown-linux-gnu -march=rv32imafd -mabi=ilp32d \
+// RUN:   -target riscv32-unknown-linux-gnu -march=rv32imafd \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-LINUX-MULTI-ILP32D %s
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -379,7 +379,9 @@
   if (const Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
 return 

[PATCH] D65634: [RISCV] Default to ilp32d/lp64d in RISC-V Linux

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Thanks @asb @lenary for the review!

I understand that, after this change, we will also want to make 
`-march=rv{32,64}gc` the default in Linux as well. Otherwise there will be an 
ABI mismatch with the default `-march=rv{32.64}i` in a default invocation.

Does this make sense?


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

https://reviews.llvm.org/D65634



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

[PATCH] D48357: [RISCV] Remove duplicated logic when determining the target ABI

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368128: [RISCV] Remove duplicated logic when determining the 
target ABI (authored by rogfer01, committed by ).
Herald added subscribers: llvm-commits, jrtc27.
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D48357?vs=212970=213804#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D48357

Files:
  cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp


Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -372,8 +372,14 @@
 }
 
 StringRef riscv::getRISCVABI(const ArgList , const llvm::Triple ) {
-  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
+  assert((Triple.getArch() == llvm::Triple::riscv32 ||
+  Triple.getArch() == llvm::Triple::riscv64) &&
+ "Unexpected triple");
+
+  if (const Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
 return A->getValue();
 
+  // FIXME: currently defaults to the soft-float ABIs. Will need to be
+  // expanded to select ilp32f, ilp32d, lp64f, lp64d when appropriate.
   return Triple.getArch() == llvm::Triple::riscv32 ? "ilp32" : "lp64";
 }
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -1848,21 +1848,11 @@
 
 void Clang::AddRISCVTargetArgs(const ArgList ,
ArgStringList ) const {
-  // FIXME: currently defaults to the soft-float ABIs. Will need to be
-  // expanded to select ilp32f, ilp32d, lp64f, lp64d when appropriate.
-  const char *ABIName = nullptr;
   const llvm::Triple  = getToolChain().getTriple();
-  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
-ABIName = A->getValue();
-  else if (Triple.getArch() == llvm::Triple::riscv32)
-ABIName = "ilp32";
-  else if (Triple.getArch() == llvm::Triple::riscv64)
-ABIName = "lp64";
-  else
-llvm_unreachable("Unexpected triple!");
+  StringRef ABIName = riscv::getRISCVABI(Args, Triple);
 
   CmdArgs.push_back("-target-abi");
-  CmdArgs.push_back(ABIName);
+  CmdArgs.push_back(ABIName.data());
 }
 
 void Clang::AddSparcTargetArgs(const ArgList ,


Index: cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -372,8 +372,14 @@
 }
 
 StringRef riscv::getRISCVABI(const ArgList , const llvm::Triple ) {
-  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
+  assert((Triple.getArch() == llvm::Triple::riscv32 ||
+  Triple.getArch() == llvm::Triple::riscv64) &&
+ "Unexpected triple");
+
+  if (const Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
 return A->getValue();
 
+  // FIXME: currently defaults to the soft-float ABIs. Will need to be
+  // expanded to select ilp32f, ilp32d, lp64f, lp64d when appropriate.
   return Triple.getArch() == llvm::Triple::riscv32 ? "ilp32" : "lp64";
 }
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -1848,21 +1848,11 @@
 
 void Clang::AddRISCVTargetArgs(const ArgList ,
ArgStringList ) const {
-  // FIXME: currently defaults to the soft-float ABIs. Will need to be
-  // expanded to select ilp32f, ilp32d, lp64f, lp64d when appropriate.
-  const char *ABIName = nullptr;
   const llvm::Triple  = getToolChain().getTriple();
-  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
-ABIName = A->getValue();
-  else if (Triple.getArch() == llvm::Triple::riscv32)
-ABIName = "ilp32";
-  else if (Triple.getArch() == llvm::Triple::riscv64)
-ABIName = "lp64";
-  else
-llvm_unreachable("Unexpected triple!");
+  StringRef ABIName = riscv::getRISCVABI(Args, Triple);
 
   CmdArgs.push_back("-target-abi");
-  CmdArgs.push_back(ABIName);
+  CmdArgs.push_back(ABIName.data());
 }
 
 void Clang::AddSparcTargetArgs(const ArgList ,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48357: [RISCV] Remove duplicated logic when determining the target ABI

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Thanks @lenary ! I will commit this shortly.


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

https://reviews.llvm.org/D48357



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

[PATCH] D65635: Sidestep false positive due to a matching git repository name

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Thanks @efriedma. I will commit this shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65635



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

[PATCH] D65635: Sidestep false positive due to a matching git repository name

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL367826: Sidestep false positive due to a matching git 
repository name (authored by rogfer01, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65635?vs=212975=213305#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65635

Files:
  cfe/trunk/test/CodeGen/constant-comparison.c


Index: cfe/trunk/test/CodeGen/constant-comparison.c
===
--- cfe/trunk/test/CodeGen/constant-comparison.c
+++ cfe/trunk/test/CodeGen/constant-comparison.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -emit-llvm %s -o - 2>&1 | not grep warning
-// RUN: %clang_cc1 -emit-llvm %s -o - | grep @b | count 1
+// RUN: %clang_cc1 -fno-ident -emit-llvm %s -o - | grep @b | count 1
 
 int a, b;
 int *c1 = 1 < 2 ?  : 


Index: cfe/trunk/test/CodeGen/constant-comparison.c
===
--- cfe/trunk/test/CodeGen/constant-comparison.c
+++ cfe/trunk/test/CodeGen/constant-comparison.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -emit-llvm %s -o - 2>&1 | not grep warning
-// RUN: %clang_cc1 -emit-llvm %s -o - | grep @b | count 1
+// RUN: %clang_cc1 -fno-ident -emit-llvm %s -o - | grep @b | count 1
 
 int a, b;
 int *c1 = 1 < 2 ?  : 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65634: [RISCV] Default to lp64d in 64-bit RISC-V Linux

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 created this revision.
rogfer01 added reviewers: asb, lenary.
Herald added subscribers: cfe-commits, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, MaskRay, 
jrtc27, shiva0217, kito-cheng, niosHD, sabuasal, apazos, simoncook, johnrusso, 
rbar.
Herald added a project: clang.

When running clang as a native compiler in 64-bit Linux RISC-V the flag 
`-mabi=lp64d` is always mandatory. This change makes it the default there.

This builds on top of D48357 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65634

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Driver/riscv64-toolchain.c
  clang/test/Preprocessor/riscv-target-features.c


Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -50,7 +50,7 @@
 
 // RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -x c -E -dM %s 
\
 // RUN: -o - | FileCheck --check-prefix=CHECK-SOFT %s
-// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -x c -E -dM %s 
\
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -mabi=lp64 -x 
c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SOFT %s
 // CHECK-SOFT: __riscv_float_abi_soft 1
 // CHECK-SOFT-NOT: __riscv_float_abi_single
@@ -66,7 +66,7 @@
 
 // RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -mabi=ilp32d 
-x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-DOUBLE %s
-// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -mabi=lp64d -x 
c -E -dM %s \
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -x c -E -dM %s 
\
 // RUN: -o - | FileCheck --check-prefix=CHECK-DOUBLE %s
 // CHECK-DOUBLE: __riscv_float_abi_double 1
 // CHECK-DOUBLE-NOT: __riscv_float_abi_soft
Index: clang/test/Driver/riscv64-toolchain.c
===
--- clang/test/Driver/riscv64-toolchain.c
+++ clang/test/Driver/riscv64-toolchain.c
@@ -68,7 +68,7 @@
 // CXX-RV64-BAREMETAL-NOSYSROOT-LP64: 
"{{.*}}/Inputs/basic_riscv64_tree/lib/gcc/riscv64-unknown-elf/8.0.1{{/|}}crtend.o"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv64-unknown-linux-gnu \
+// RUN:   -target riscv64-unknown-linux-gnu -mabi=lp64 \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-LINUX-MULTI-LP64 %s
@@ -84,7 +84,7 @@
 // C-RV64-LINUX-MULTI-LP64: 
"-L{{.*}}/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib64/lp64"
 
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
-// RUN:   -target riscv64-unknown-linux-gnu -march=rv64imafd -mabi=lp64d \
+// RUN:   -target riscv64-unknown-linux-gnu -march=rv64imafd \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
 // RUN:   --sysroot=%S/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV64-LINUX-MULTI-LP64D %s
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -379,7 +379,8 @@
   if (const Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
 return A->getValue();
 
-  // FIXME: currently defaults to the soft-float ABIs. Will need to be
-  // expanded to select ilp32f, ilp32d, lp64f, lp64d when appropriate.
-  return Triple.getArch() == llvm::Triple::riscv32 ? "ilp32" : "lp64";
+  // 64-bit RISC-V Linux defaults to lp64d.
+  return Triple.getArch() == llvm::Triple::riscv32
+ ? "ilp32"
+ : Triple.getOS() == llvm::Triple::Linux ? "lp64d" : "lp64";
 }


Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -50,7 +50,7 @@
 
 // RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SOFT %s
-// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -x c -E -dM %s \
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -mabi=lp64 -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SOFT %s
 // CHECK-SOFT: __riscv_float_abi_soft 1
 // CHECK-SOFT-NOT: __riscv_float_abi_single
@@ -66,7 +66,7 @@
 
 // RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -mabi=ilp32d -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-DOUBLE %s
-// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -mabi=lp64d -x c -E -dM %s \
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -x c -E -dM %s \
 // 

[PATCH] D65635: Sidestep false positive due to a matching git repository name

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 created this revision.
rogfer01 added reviewers: eli.friedman, ddunbar.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
rogfer01 edited reviewers, added: efriedma; removed: eli.friedman.

I have failures in this test because the `grep @b` gets confused by the `clang 
version` including a repository name like this

  !1 = !{!"clang version 10.0.0 (git@build-machine:llvm/llvm-monorepo.git 
fe958c0e8c89ec663c8e551936778e2cbb460154)"}

I considered something like `grep -w` but my understanding of the manpages was 
that that isn't super portable. So I think it is easier to make clang not to 
output that metadata using `-fno-ident`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65635

Files:
  clang/test/CodeGen/constant-comparison.c


Index: clang/test/CodeGen/constant-comparison.c
===
--- clang/test/CodeGen/constant-comparison.c
+++ clang/test/CodeGen/constant-comparison.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -emit-llvm %s -o - 2>&1 | not grep warning
-// RUN: %clang_cc1 -emit-llvm %s -o - | grep @b | count 1
+// RUN: %clang_cc1 -fno-ident -emit-llvm %s -o - | grep @b | count 1
 
 int a, b;
 int *c1 = 1 < 2 ?  : 


Index: clang/test/CodeGen/constant-comparison.c
===
--- clang/test/CodeGen/constant-comparison.c
+++ clang/test/CodeGen/constant-comparison.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -emit-llvm %s -o - 2>&1 | not grep warning
-// RUN: %clang_cc1 -emit-llvm %s -o - | grep @b | count 1
+// RUN: %clang_cc1 -fno-ident -emit-llvm %s -o - | grep @b | count 1
 
 int a, b;
 int *c1 = 1 < 2 ?  : 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48357: [RISCV] Remove duplicated logic when determining the target ABI

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 updated this revision to Diff 212970.
rogfer01 added a comment.

ChangeLog

- Rebase change


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

https://reviews.llvm.org/D48357

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1841,21 +1841,11 @@
 
 void Clang::AddRISCVTargetArgs(const ArgList ,
ArgStringList ) const {
-  // FIXME: currently defaults to the soft-float ABIs. Will need to be
-  // expanded to select ilp32f, ilp32d, lp64f, lp64d when appropriate.
-  const char *ABIName = nullptr;
   const llvm::Triple  = getToolChain().getTriple();
-  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
-ABIName = A->getValue();
-  else if (Triple.getArch() == llvm::Triple::riscv32)
-ABIName = "ilp32";
-  else if (Triple.getArch() == llvm::Triple::riscv64)
-ABIName = "lp64";
-  else
-llvm_unreachable("Unexpected triple!");
+  StringRef ABIName = riscv::getRISCVABI(Args, Triple);
 
   CmdArgs.push_back("-target-abi");
-  CmdArgs.push_back(ABIName);
+  CmdArgs.push_back(ABIName.data());
 }
 
 void Clang::AddSparcTargetArgs(const ArgList ,
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -372,8 +372,14 @@
 }
 
 StringRef riscv::getRISCVABI(const ArgList , const llvm::Triple ) {
-  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
+  assert((Triple.getArch() == llvm::Triple::riscv32 ||
+  Triple.getArch() == llvm::Triple::riscv64) &&
+ "Unexpected triple");
+
+  if (const Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
 return A->getValue();
 
+  // FIXME: currently defaults to the soft-float ABIs. Will need to be
+  // expanded to select ilp32f, ilp32d, lp64f, lp64d when appropriate.
   return Triple.getArch() == llvm::Triple::riscv32 ? "ilp32" : "lp64";
 }


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1841,21 +1841,11 @@
 
 void Clang::AddRISCVTargetArgs(const ArgList ,
ArgStringList ) const {
-  // FIXME: currently defaults to the soft-float ABIs. Will need to be
-  // expanded to select ilp32f, ilp32d, lp64f, lp64d when appropriate.
-  const char *ABIName = nullptr;
   const llvm::Triple  = getToolChain().getTriple();
-  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
-ABIName = A->getValue();
-  else if (Triple.getArch() == llvm::Triple::riscv32)
-ABIName = "ilp32";
-  else if (Triple.getArch() == llvm::Triple::riscv64)
-ABIName = "lp64";
-  else
-llvm_unreachable("Unexpected triple!");
+  StringRef ABIName = riscv::getRISCVABI(Args, Triple);
 
   CmdArgs.push_back("-target-abi");
-  CmdArgs.push_back(ABIName);
+  CmdArgs.push_back(ABIName.data());
 }
 
 void Clang::AddSparcTargetArgs(const ArgList ,
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -372,8 +372,14 @@
 }
 
 StringRef riscv::getRISCVABI(const ArgList , const llvm::Triple ) {
-  if (Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
+  assert((Triple.getArch() == llvm::Triple::riscv32 ||
+  Triple.getArch() == llvm::Triple::riscv64) &&
+ "Unexpected triple");
+
+  if (const Arg *A = Args.getLastArg(options::OPT_mabi_EQ))
 return A->getValue();
 
+  // FIXME: currently defaults to the soft-float ABIs. Will need to be
+  // expanded to select ilp32f, ilp32d, lp64f, lp64d when appropriate.
   return Triple.getArch() == llvm::Triple::riscv32 ? "ilp32" : "lp64";
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48357: [RISCV] Remove duplicated logic when determining the target ABI

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Hi @lenary, sure I can rebase this.

However, I think it may be better to do the `lp64d` change in another phab so 
we can keep this one NFC.


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

https://reviews.llvm.org/D48357



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

[PATCH] D60456: [RISCV] Hard float ABI support

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

> As noted in another comment, it's not entirely clear what zero-width bitfield 
> behaviour to match (see here 
> )
>  as GCC seems buggy and the ABI is under-specified. Ideally I'd like to land 
> this patch and follow-up to adjust the zero-width bitfield behaviour if 
> necessary once that psABI issue is resolved.

Agreed, I presume the original intent in the psABI was to have C and C++ behave 
the same. We're siding with gcc in this patch but it should not be difficult to 
change if the psABI resolves this in favour of g++.


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

https://reviews.llvm.org/D60456



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

[PATCH] D60456: [RISCV] Hard float ABI support

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:9352
+return false;
+  // As a quirk of the ABI, zero-width bitfields aren't ignored for fp+fp
+  // or int+fp structs, but are ignored for a struct with an fp field and

I found some mismatch in behaviour between gcc and g++ that we may want to 
address in the psABI first.

For instance, given the following struct (I'm using gcc 8.3.0)

```lang=cpp
// t.c
struct A
{
  int :0;
  double d;
  int :0;
  long x;
  int :0;
};

extern void bar(struct A);
void foo(struct A a)
{
  a.d =- a.d;
  a.x += 1;
  return bar(a);
}
```

we are emitting this

```
$ clang --target=riscv64 -march=rv64gc -mabi=lp64d -S -o-  t.c -O2  
...
foo:# @foo
# %bb.0:# %entry
addia2, zero, -1
sllia2, a2, 63
xor a0, a0, a2
addia1, a1, 1
tailbar
```

which matches with what g++ does (i.e in both cases `a0` is `a.d` and `a1` is 
`a.x`)

```
$ ./riscv64-unknown-linux-gnu-g++ -S -O2 -o- -x c test.cc
...
foo:
fmv.d.x fa5,a0
addisp,sp,-16
fneg.d  fa5,fa5
addia1,a1,1
addisp,sp,16
fmv.x.d a0,fa5
tailbar
```

But I found a mismatch while using C++. Clang emits the same for C and C++ 
(modulo `.cfi` stuff)

```
$ clang --target=riscv64 -march=rv64gc -mabi=lp64d -S -o-  -x c++ t.c -O2  
_Z3foo1A:   # @_Z3foo1A
.cfi_startproc
# %bb.0:# %entry
addia2, zero, -1
sllia2, a2, 63
xor a0, a0, a2
addia1, a1, 1
.cfi_def_cfa_offset 0
tail_Z3bar1A
```

But g++ seems to ignore the zero-width bitfields: `fa0` is  `a.d` and `a0` is 
`a.x`

```
$ riscv64-unknown-linux-gnu-g++  -S -O2 -x c++ t.c -o-
...
_Z3foo1A:
.LFB0:
.cfi_startproc
fneg.d  fa0,fa0
addisp,sp,-16
.cfi_def_cfa_offset 16
addia0,a0,1
addisp,sp,16
.cfi_def_cfa_offset 0
tail_Z3bar1A
.cfi_endproc
```

This is a bit worrying as it might complicate interoperability between C and 
C++ (I tried wrapping everything inside an `extern "C"` just in case but it 
didn't change g++'s behaviour).

Do you mind to confirm this issue?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:9394
+// Call getCoerceAndExpand for the two-element flattened struct described by
+// Field1Ty, Filed2Ty, Filed2Off. This method will create an appropriate
+// coerceToType and unpaddedCoerceToType.

Typo  in `Filed2Ty` and `Filed2Off`


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

https://reviews.llvm.org/D60456



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

[PATCH] D59298: [RISCV] Pass -target-abi to -cc1as

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356981: [RISCV] Pass -target-abi to -cc1as (authored by 
rogfer01, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59298?vs=190409=192248#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59298

Files:
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.h
  cfe/trunk/test/Driver/riscv-abi.c
  cfe/trunk/tools/driver/cc1as_main.cpp

Index: cfe/trunk/test/Driver/riscv-abi.c
===
--- cfe/trunk/test/Driver/riscv-abi.c
+++ cfe/trunk/test/Driver/riscv-abi.c
@@ -2,6 +2,10 @@
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -mabi=ilp32 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
+// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
+// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o \
+// RUN:   -mabi=ilp32 2>&1 | FileCheck -check-prefix=CHECK-ILP32 %s
 
 // CHECK-ILP32: "-target-abi" "ilp32"
 
@@ -26,6 +30,10 @@
 // RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
 // RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -mabi=lp64 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
+// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LP64  %s
+// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o \
+// RUN:   -mabi=lp64 2>&1 | FileCheck -check-prefix=CHECK-LP64 %s
 
 // CHECK-LP64: "-target-abi" "lp64"
 
Index: cfe/trunk/lib/Driver/ToolChains/Clang.h
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.h
+++ cfe/trunk/lib/Driver/ToolChains/Clang.h
@@ -119,6 +119,8 @@
  llvm::opt::ArgStringList ) const;
   void AddX86TargetArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const;
+  void AddRISCVTargetArgs(const llvm::opt::ArgList ,
+  llvm::opt::ArgStringList ) const;
   bool hasGoodDiagnostics() const override { return true; }
   bool hasIntegratedAssembler() const override { return false; }
   bool hasIntegratedCPP() const override { return false; }
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -5931,6 +5931,15 @@
   }
 }
 
+void ClangAs::AddRISCVTargetArgs(const ArgList ,
+   ArgStringList ) const {
+  const llvm::Triple  = getToolChain().getTriple();
+  StringRef ABIName = riscv::getRISCVABI(Args, Triple);
+
+  CmdArgs.push_back("-target-abi");
+  CmdArgs.push_back(ABIName.data());
+}
+
 void ClangAs::ConstructJob(Compilation , const JobAction ,
const InputInfo , const InputInfoList ,
const ArgList ,
@@ -6100,6 +6109,11 @@
 CmdArgs.push_back("-arm-add-build-attributes");
 }
 break;
+
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:
+AddRISCVTargetArgs(Args, CmdArgs);
+break;
   }
 
   // Consume all the warning flags. Usually this would be handled more
Index: cfe/trunk/tools/driver/cc1as_main.cpp
===
--- cfe/trunk/tools/driver/cc1as_main.cpp
+++ cfe/trunk/tools/driver/cc1as_main.cpp
@@ -137,6 +137,10 @@
   /// The name of the relocation model to use.
   std::string RelocationModel;
 
+  /// The ABI targeted by the backend. Specified using -target-abi. Empty
+  /// otherwise.
+  std::string TargetABI;
+
   /// @}
 
 public:
@@ -282,6 +286,7 @@
   Opts.NoExecStack = Args.hasArg(OPT_mno_exec_stack);
   Opts.FatalWarnings = Args.hasArg(OPT_massembler_fatal_warnings);
   Opts.RelocationModel = Args.getLastArgValue(OPT_mrelocation_model, "pic");
+  Opts.TargetABI = Args.getLastArgValue(OPT_target_abi);
   Opts.IncrementalLinkerCompatible =
   Args.hasArg(OPT_mincremental_linker_compatible);
   Opts.SymbolDefs = Args.getAllArgValues(OPT_defsym);
@@ -426,6 +431,9 @@
   raw_pwrite_stream *Out = FDOS.get();
   std::unique_ptr BOS;
 
+  MCTargetOptions MCOptions;
+  MCOptions.ABIName = Opts.TargetABI;
+
   // FIXME: There is a bit of code duplication with addPassesToEmitFile.
   if (Opts.OutputType == AssemblerInvocation::FT_Asm) {
 MCInstPrinter *IP = TheTarget->createMCInstPrinter(
@@ -434,7 +442,6 @@
 std::unique_ptr CE;
 if (Opts.ShowEncoding)
   CE.reset(TheTarget->createMCCodeEmitter(*MCII, *MRI, Ctx));
-MCTargetOptions MCOptions;
 std::unique_ptr MAB(
 TheTarget->createMCAsmBackend(*STI, 

[PATCH] D59298: [RISCV] Pass -target-abi to -cc1as

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Thanks Alex. I will commit it shortly.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59298



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

[PATCH] D48357: [RISCV] Remove duplicated logic when determining the target ABI

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.
Herald added subscribers: jocewei, PkmX, rkruppe.

@asb in D59298  I call `riscv::getRISCVABI` 
for `ClangAs`, does it make sense to make the same change for `Clang` here?

Thank you.


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

https://reviews.llvm.org/D48357



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

[PATCH] D59298: [RISCV] Pass -target-abi to -cc1as

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 created this revision.
rogfer01 added a reviewer: asb.
Herald added subscribers: cfe-commits, jocewei, PkmX, rkruppe, the_o, 
brucehoult, MartinMosbeck, edward-jones, zzheng, jrtc27, shiva0217, kito-cheng, 
niosHD, sabuasal, apazos, simoncook, johnrusso, rbar.
Herald added a project: clang.

The RISC-V assembler needs the target ABI because it defines a flag of the the 
ELF file, as described in [1].

Make `clang` (the driver) to pass the target ABI to `-cc1as` in exactly the 
same way it does for `-cc1`.

Currently `-cc1as` knows about `-target-abi` but is not handling it. Handle it 
and pass it to the MC layer via MCTargetOptions.

[1] 
https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#file-header


Repository:
  rC Clang

https://reviews.llvm.org/D59298

Files:
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Clang.h
  test/Driver/riscv-abi.c
  tools/driver/cc1as_main.cpp

Index: tools/driver/cc1as_main.cpp
===
--- tools/driver/cc1as_main.cpp
+++ tools/driver/cc1as_main.cpp
@@ -137,6 +137,10 @@
   /// The name of the relocation model to use.
   std::string RelocationModel;
 
+  /// The ABI targeted by the backend. Specified using -target-abi. Empty
+  /// otherwise.
+  std::string TargetABI;
+
   /// @}
 
 public:
@@ -282,6 +286,7 @@
   Opts.NoExecStack = Args.hasArg(OPT_mno_exec_stack);
   Opts.FatalWarnings = Args.hasArg(OPT_massembler_fatal_warnings);
   Opts.RelocationModel = Args.getLastArgValue(OPT_mrelocation_model, "pic");
+  Opts.TargetABI = Args.getLastArgValue(OPT_target_abi);
   Opts.IncrementalLinkerCompatible =
   Args.hasArg(OPT_mincremental_linker_compatible);
   Opts.SymbolDefs = Args.getAllArgValues(OPT_defsym);
@@ -426,6 +431,9 @@
   raw_pwrite_stream *Out = FDOS.get();
   std::unique_ptr BOS;
 
+  MCTargetOptions MCOptions;
+  MCOptions.ABIName = Opts.TargetABI;
+
   // FIXME: There is a bit of code duplication with addPassesToEmitFile.
   if (Opts.OutputType == AssemblerInvocation::FT_Asm) {
 MCInstPrinter *IP = TheTarget->createMCInstPrinter(
@@ -434,7 +442,6 @@
 std::unique_ptr CE;
 if (Opts.ShowEncoding)
   CE.reset(TheTarget->createMCCodeEmitter(*MCII, *MRI, Ctx));
-MCTargetOptions MCOptions;
 std::unique_ptr MAB(
 TheTarget->createMCAsmBackend(*STI, *MRI, MCOptions));
 
@@ -455,7 +462,6 @@
 
 std::unique_ptr CE(
 TheTarget->createMCCodeEmitter(*MCII, *MRI, Ctx));
-MCTargetOptions MCOptions;
 std::unique_ptr MAB(
 TheTarget->createMCAsmBackend(*STI, *MRI, MCOptions));
 std::unique_ptr OW =
@@ -489,9 +495,8 @@
   createMCAsmParser(SrcMgr, Ctx, *Str.get(), *MAI));
 
   // FIXME: init MCTargetOptions from sanitizer flags here.
-  MCTargetOptions Options;
   std::unique_ptr TAP(
-  TheTarget->createMCAsmParser(*STI, *Parser, *MCII, Options));
+  TheTarget->createMCAsmParser(*STI, *Parser, *MCII, MCOptions));
   if (!TAP)
 Failed = Diags.Report(diag::err_target_unknown_triple) << Opts.Triple;
 
Index: test/Driver/riscv-abi.c
===
--- test/Driver/riscv-abi.c
+++ test/Driver/riscv-abi.c
@@ -2,6 +2,10 @@
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -mabi=ilp32 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
+// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
+// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o \
+// RUN:   -mabi=ilp32 2>&1 | FileCheck -check-prefix=CHECK-ILP32 %s
 
 // CHECK-ILP32: "-target-abi" "ilp32"
 
@@ -26,6 +30,10 @@
 // RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
 // RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -mabi=lp64 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
+// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LP64  %s
+// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o \
+// RUN:   -mabi=lp64 2>&1 | FileCheck -check-prefix=CHECK-LP64 %s
 
 // CHECK-LP64: "-target-abi" "lp64"
 
Index: lib/Driver/ToolChains/Clang.h
===
--- lib/Driver/ToolChains/Clang.h
+++ lib/Driver/ToolChains/Clang.h
@@ -119,6 +119,8 @@
  llvm::opt::ArgStringList ) const;
   void AddX86TargetArgs(const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ) const;
+  void AddRISCVTargetArgs(const llvm::opt::ArgList ,
+  llvm::opt::ArgStringList ) const;
   bool hasGoodDiagnostics() const override { return true; }
   bool hasIntegratedAssembler() const override { return false; }
   bool hasIntegratedCPP() const override { return false; }
Index: lib/Driver/ToolChains/Clang.cpp

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Looks sensible to me.

I'm just curious why we want to prevent emission of atomic LLVM instructions at 
this point. Won't LLVM's AtomicExpand perform a similar lowering already? 
Perhaps the goal is to save that pass some work?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57450



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

[PATCH] D48589: [WIP] [CodeGen] Allow specifying Extend to CoerceAndExpand

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 abandoned this revision.
rogfer01 added a comment.

Upstream 

 recently amended the ABI spec so it looks to me this is not going to be needed.


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

https://reviews.llvm.org/D48589



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

[PATCH] D51972: [RISCV] Explicitly set an empty --sysroot in the test

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC342060: [RISCV] Explicitly set an empty --sysroot in the 
test (authored by rogfer01, committed by ).
Herald added a subscriber: jrtc27.

Repository:
  rC Clang

https://reviews.llvm.org/D51972

Files:
  test/Driver/riscv32-toolchain.c


Index: test/Driver/riscv32-toolchain.c
===
--- test/Driver/riscv32-toolchain.c
+++ test/Driver/riscv32-toolchain.c
@@ -21,6 +21,7 @@
 
 // RUN: %clang %s -### -no-canonical-prefixes \
 // RUN:   -target riscv32-unknown-elf \
+// RUN:   --sysroot= \
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-BAREMETAL-NOSYSROOT-ILP32 %s
 
@@ -52,6 +53,7 @@
 
 // RUN: %clangxx %s -### -no-canonical-prefixes \
 // RUN:   -target riscv32-unknown-elf -stdlib=libstdc++ \
+// RUN:   --sysroot= \
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree 2>&1 \
 // RUN:   | FileCheck -check-prefix=CXX-RV32-BAREMETAL-NOSYSROOT-ILP32 %s
 


Index: test/Driver/riscv32-toolchain.c
===
--- test/Driver/riscv32-toolchain.c
+++ test/Driver/riscv32-toolchain.c
@@ -21,6 +21,7 @@
 
 // RUN: %clang %s -### -no-canonical-prefixes \
 // RUN:   -target riscv32-unknown-elf \
+// RUN:   --sysroot= \
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree 2>&1 \
 // RUN:   | FileCheck -check-prefix=C-RV32-BAREMETAL-NOSYSROOT-ILP32 %s
 
@@ -52,6 +53,7 @@
 
 // RUN: %clangxx %s -### -no-canonical-prefixes \
 // RUN:   -target riscv32-unknown-elf -stdlib=libstdc++ \
+// RUN:   --sysroot= \
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree 2>&1 \
 // RUN:   | FileCheck -check-prefix=CXX-RV32-BAREMETAL-NOSYSROOT-ILP32 %s
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51972: [RISCV] Explicitly set an empty --sysroot in the test

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Thanks I will do it shortly.


https://reviews.llvm.org/D51972



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

[PATCH] D51972: [RISCV] Explicitly set an empty --sysroot in the test

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

I can commit it.


https://reviews.llvm.org/D51972



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

[PATCH] D51972: [RISCV] Explicitly set an empty --sysroot in the test

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

rogfer01 added a comment.

Thanks for the review!


https://reviews.llvm.org/D51972



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