[PATCH] D93591: [MLIR][SPIRV] Add (de-)serialization support for SpecConstantOpeation.

[Kareem Ergawy via Phabricator via cfe-commits]

ergawy updated this revision to Diff 315658.
ergawy marked 5 inline comments as done.
ergawy added a comment.

Handle review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93591

Files:
  mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td
  mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
  mlir/lib/Target/SPIRV/Deserialization.cpp
  mlir/lib/Target/SPIRV/Serialization.cpp
  mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
  mlir/test/Target/SPIRV/spec-constant.mlir

Index: mlir/test/Target/SPIRV/spec-constant.mlir
===
--- mlir/test/Target/SPIRV/spec-constant.mlir
+++ mlir/test/Target/SPIRV/spec-constant.mlir
@@ -85,3 +85,34 @@
   // CHECK: spv.specConstantComposite @scc_vector (@sc_f32_1, @sc_f32_2, @sc_f32_3) : vector<3xf32>
   spv.specConstantComposite @scc_vector (@sc_f32_1, @sc_f32_2, @sc_f32_3) : vector<3 x f32>
 }
+
+// -
+
+spv.module Logical GLSL450 requires #spv.vce {
+
+  spv.specConstant @sc_i32_1 = 1 : i32
+
+  spv.func @use_composite() -> (i32) "None" {
+// CHECK: [[USE1:%.*]] = spv.mlir.referenceof @sc_i32_1 : i32
+// CHECK: [[USE2:%.*]] = spv.constant 0 : i32
+
+// CHECK: [[RES1:%.*]] = spv.SpecConstantOperation wraps "spv.ISub"([[USE1]], [[USE2]]) : (i32, i32) -> i32
+
+// CHECK: [[USE3:%.*]] = spv.mlir.referenceof @sc_i32_1 : i32
+// CHECK: [[USE4:%.*]] = spv.constant 0 : i32
+
+// CHECK: [[RES2:%.*]] = spv.SpecConstantOperation wraps "spv.ISub"([[USE3]], [[USE4]]) : (i32, i32) -> i32
+
+%0 = spv.mlir.referenceof @sc_i32_1 : i32
+%1 = spv.constant 0 : i32
+%2 = spv.SpecConstantOperation wraps "spv.ISub"(%0, %1) : (i32, i32) -> i32
+
+// CHECK: [[RES3:%.*]] = spv.SpecConstantOperation wraps "spv.IMul"([[RES1]], [[RES2]]) : (i32, i32) -> i32
+%3 = spv.SpecConstantOperation wraps "spv.IMul"(%2, %2) : (i32, i32) -> i32
+
+// Make sure deserialization continues from the right place after creating
+// the previous op.
+// CHECK: spv.ReturnValue [[RES3]]
+spv.ReturnValue %3 : i32
+  }
+}
Index: mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
===
--- mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
+++ mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
@@ -780,6 +780,20 @@
 
 // -
 
+spv.module Logical GLSL450 {
+  spv.specConstant @sc = 42 : i32
+
+  spv.func @foo() -> i32 "None" {
+// CHECK: [[SC:%.*]] = spv.mlir.referenceof @sc
+%0 = spv.mlir.referenceof @sc : i32
+// CHECK: spv.SpecConstantOperation wraps "spv.ISub"([[SC]], [[SC]]) : (i32, i32) -> i32
+%1 = spv.SpecConstantOperation wraps "spv.ISub"(%0, %0) : (i32, i32) -> i32
+spv.ReturnValue %1 : i32
+  }
+}
+
+// -
+
 spv.module Logical GLSL450 {
   spv.func @foo() -> i32 "None" {
 %0 = spv.constant 1: i32
Index: mlir/lib/Target/SPIRV/Serialization.cpp
===
--- mlir/lib/Target/SPIRV/Serialization.cpp
+++ mlir/lib/Target/SPIRV/Serialization.cpp
@@ -204,6 +204,9 @@
   LogicalResult
   processSpecConstantCompositeOp(spirv::SpecConstantCompositeOp op);
 
+  LogicalResult
+  processSpecConstantOperationOp(spirv::SpecConstantOperationOp op);
+
   /// SPIR-V dialect supports OpUndef using spv.UndefOp that produces a SSA
   /// value to use with other operations. The SPIR-V spec recommends that
   /// OpUndef be generated at module level. The serialization generates an
@@ -711,6 +714,49 @@
   return processName(resultID, op.sym_name());
 }
 
+LogicalResult
+Serializer::processSpecConstantOperationOp(spirv::SpecConstantOperationOp op) {
+  uint32_t typeID = 0;
+  if (failed(processType(op.getLoc(), op.getType(), typeID))) {
+return failure();
+  }
+
+  auto resultID = getNextID();
+
+  SmallVector operands;
+  operands.push_back(typeID);
+  operands.push_back(resultID);
+
+  Block &block = op.getRegion().getBlocks().front();
+  Operation &enclosedOp = block.getOperations().front();
+
+  std::string enclosedOpName;
+  llvm::raw_string_ostream rss(enclosedOpName);
+  rss << "Op" << enclosedOp.getName().stripDialect();
+  auto enclosedOpcode = spirv::symbolizeOpcode(rss.str());
+
+  if (!enclosedOpcode) {
+op.emitError("Couldn't find op code for op ")
+<< enclosedOp.getName().getStringRef();
+return failure();
+  }
+
+  operands.push_back(static_cast(enclosedOpcode.getValue()));
+
+  // Append operands to the enclosed op to the list of operands.
+  for (Value operand : enclosedOp.getOperands()) {
+uint32_t id = getValueID(operand);
+assert(id && "use before def!");
+operands.push_back(id);
+  }
+
+  encodeInstructionInto(typesGlobalValues,
+spirv::Opcode::OpSpecConstantOperation, operands);
+  valueIDMap[op.getResult()] = resultID;
+
+  return success();
+}
+
 LogicalResult Serializer::processUndefOp(spirv::UndefOp op) {
   auto undefType = op.ge

[PATCH] D93591: [MLIR][SPIRV] Add (de-)serialization support for SpecConstantOpeation.

[Kareem Ergawy via Phabricator via cfe-commits]

ergawy marked 2 inline comments as done.
ergawy added inline comments.



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:623
 {"referencesProvider", true},
-{"astProvider", true},
+{"astProvider", true}, // clangd extension
 {"executeCommandProvider",

antiagainst wrote:
> Accidental change or rebase leftover?
> 
> Anyway we need to revert the change here. :)
No idea where that came from. Rebasing made it go away.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93591

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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

[Lukas Barth via Phabricator via cfe-commits]

tinloaf updated this revision to Diff 315664.
tinloaf marked an inline comment as done.
tinloaf added a comment.

- Unify the type of the options for `AlignConsecutive`{`BitFields`, 
`Declarations`, `Assignments`, `Macros`}
- Implement across-alignment for bit fields, declarations, assignments and 
macros
- Add tests for across-alignment for bit fields, declarations, assignments and 
macros
- Add changelog entry


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11330,8 +11330,8 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
-  Tab.AlignConsecutiveDeclarations = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Tab.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
   verifyFormat("class Assign {\n"
@@ -11569,8 +11569,8 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
-  Tab.AlignConsecutiveDeclarations = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Tab.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
   verifyFormat("class Assign {\n"
@@ -12271,9 +12271,9 @@
 
 TEST_F(FormatTest, AlignConsecutiveMacros) {
   FormatStyle Style = getLLVMStyle();
-  Style.AlignConsecutiveAssignments = true;
-  Style.AlignConsecutiveDeclarations = true;
-  Style.AlignConsecutiveMacros = false;
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_None;
 
   verifyFormat("#define a 3\n"
"#define  4\n"
@@ -12297,7 +12297,7 @@
"#define (x, y) (x - y)",
Style);
 
-  Style.AlignConsecutiveMacros = true;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_Consecutive;
   verifyFormat("#define a3\n"
"#define  4\n"
"#define ccc  (5)",
@@ -12337,7 +12337,7 @@
"};",
Style);
 
-  Style.AlignConsecutiveMacros = false;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_None;
   Style.ColumnLimit = 20;
 
   verifyFormat("#define a  \\\n"
@@ -12351,7 +12351,7 @@
"  \"\"\n",
Style);
 
-  Style.AlignConsecutiveMacros = true;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_Consecutive;
   verifyFormat("#define a  \\\n"
"  \"aa\"\n"
"#define D  \\\n"
@@ -12362,12 +12362,766 @@
"  \"F\"  \\\n"
"  \"\"\n",
Style);
+
+  // Test across comments
+  Style.MaxEmptyLinesToKeep = 10;
+  Style.ReflowComments = false;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_AcrossComments;
+  EXPECT_EQ("#define a3\n"
+"// line comment\n"
+"#define  4\n"
+"#define ccc  (5)",
+format("#define a 3\n"
+   "// line comment\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style));
+
+  EXPECT_EQ("#define a3\n"
+"/* block comment */\n"
+"#define  4\n"
+"#define ccc  (5)",
+format("#define a  3\n"
+   "/* block comment */\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style));
+
+  EXPECT_EQ("#define a3\n"
+"/* multi-line *\n"
+" * block comment */\n"
+"#define  4\n"
+"#define ccc  (5)",
+format("#define a 3\n"
+   "/* multi-line *\n"
+   " * block comment */\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style));
+
+  EXPECT_EQ("#define a3\n"
+"// multi-line line comment\n"
+"//\n"
+"#define  4\n"
+"#define ccc  (5)",
+format("#define a  3\n"
+   "// multi-line line comment\n"
+   "//\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style));
+
+  EXPECT_EQ("#define a 3\n"
+"// empty lines still break.\n"
+"\n"
+"#define  4\n"

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

[Lukas Barth via Phabricator via cfe-commits]

tinloaf added a comment.

In D93986#2477860 , 
@HazardyKnusperkeks wrote:

> In D93986#2477474 , @tinloaf wrote:
>
>> In D93986#2477383 , @MyDeveloperDay 
>> wrote:
>>
>>> Ideally we should add a comment to the release notes, (which you could do 
>>> via a separate NFC commit if you wanted)
>>
>> Thank, I'll do that. But before that, I will create a revision that adds the 
>> same feature for the other three alignment types.
>
> I think it should happen in this revision so that it is atomically.

Done - this revision now also includes the functionality for bit fields, macros 
and declarations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

[Lukas Barth via Phabricator via cfe-commits]

tinloaf marked 3 inline comments as done.
tinloaf added a comment.

This is now an all-in-one revision, including bit fields, assignments, 
declarations and macros. I did not reproduce the full "across empty lines, 
across comments, across empty lines *and* comments" test suite for all four of 
them, since they all use the same logic (and code, mostly). 
`AlignConsecutiveAssignments` is tested extensively, 
`AlignConsecutiveBitFields` and `AlignConsecutiveDeclarations` only have tests 
for `AlignAcrossEmptyLinesAndComments` (since they delegate to the same 
`AlignTokens` method), and `AlignConsecutiveMacros` has its own set of tests, 
since it uses a different implementation (of the same logic, basically).




Comment at: clang/docs/ClangFormatStyleOptions.rst:198
 
-**AlignConsecutiveAssignments** (``bool``)
-  If ``true``, aligns consecutive assignments.
+**AlignConsecutiveAssignments** (``AlignConsecutiveAssignmentsStyle``)
+  Style of aligning assignments on consecutive lines.

MyDeveloperDay wrote:
> tinloaf wrote:
> > MyDeveloperDay wrote:
> > > As you plan to do the other cases won't this introduce
> > > 
> > > `AlignBitFieldStyle`
> > > and
> > > `AlignMacroStyle`
> > > 
> > > why not have 1  `AlignConsecutiveStyle` that can be used for all 3, then 
> > > you don't need the other enum `TokenAlignmentStyle ` which means you 
> > > won't need to map from one to the other?
> > Yes, it would. I was under the assumption that every multiple-choice option 
> > in the options should have its own enum, for type safety. But yes, that 
> > would be way more elegant (and make the diff much smaller). I'll change it!
> we don't make that distinction for those options that use true/false so I'm 
> not sure why enums need to be any different
> 
> it may require some changes to the doc/tools/dump_style.py to ensure it gives 
> meaningful documentation, but some consistency between the values might help 
> a little (especially as we'd like to use the same AlignTokens algortihm)
> 
> You'd end up with 3 functions to map the various styles to you 
> TokenAlignmentStyle anyway, and hence you'd lose the type safety
> 
> To be honest I get a bit annoyed at the use of "None" "Never" "false", "No" 
> anyway, so I don't feel we are in a great place in terms of consistency.
I have unified the type of the style options. I think the solution for the 
documentation is Okay, though not ideal. Each of the options' docs now starts 
with an example including the relevant tokens (e.g., bit field ':'s for 
`AlignConsecutiveBitFields`), but the examples illustrating the individual 
options always use assignments. I'm not sure how I could work around this using 
only a single type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

[Lukas Barth via Phabricator via cfe-commits]

tinloaf updated this revision to Diff 315665.
tinloaf added a comment.

Fix merge conflict in the release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11330,8 +11330,8 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
-  Tab.AlignConsecutiveDeclarations = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Tab.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
   verifyFormat("class Assign {\n"
@@ -11569,8 +11569,8 @@
"*/\n"
"}",
Tab));
-  Tab.AlignConsecutiveAssignments = true;
-  Tab.AlignConsecutiveDeclarations = true;
+  Tab.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Tab.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
   Tab.TabWidth = 4;
   Tab.IndentWidth = 4;
   verifyFormat("class Assign {\n"
@@ -12405,9 +12405,9 @@
 
 TEST_F(FormatTest, AlignConsecutiveMacros) {
   FormatStyle Style = getLLVMStyle();
-  Style.AlignConsecutiveAssignments = true;
-  Style.AlignConsecutiveDeclarations = true;
-  Style.AlignConsecutiveMacros = false;
+  Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations = FormatStyle::ACS_Consecutive;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_None;
 
   verifyFormat("#define a 3\n"
"#define  4\n"
@@ -12431,7 +12431,7 @@
"#define (x, y) (x - y)",
Style);
 
-  Style.AlignConsecutiveMacros = true;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_Consecutive;
   verifyFormat("#define a3\n"
"#define  4\n"
"#define ccc  (5)",
@@ -12471,7 +12471,7 @@
"};",
Style);
 
-  Style.AlignConsecutiveMacros = false;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_None;
   Style.ColumnLimit = 20;
 
   verifyFormat("#define a  \\\n"
@@ -12485,7 +12485,7 @@
"  \"\"\n",
Style);
 
-  Style.AlignConsecutiveMacros = true;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_Consecutive;
   verifyFormat("#define a  \\\n"
"  \"aa\"\n"
"#define D  \\\n"
@@ -12496,12 +12496,766 @@
"  \"F\"  \\\n"
"  \"\"\n",
Style);
+
+  // Test across comments
+  Style.MaxEmptyLinesToKeep = 10;
+  Style.ReflowComments = false;
+  Style.AlignConsecutiveMacros = FormatStyle::ACS_AcrossComments;
+  EXPECT_EQ("#define a3\n"
+"// line comment\n"
+"#define  4\n"
+"#define ccc  (5)",
+format("#define a 3\n"
+   "// line comment\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style));
+
+  EXPECT_EQ("#define a3\n"
+"/* block comment */\n"
+"#define  4\n"
+"#define ccc  (5)",
+format("#define a  3\n"
+   "/* block comment */\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style));
+
+  EXPECT_EQ("#define a3\n"
+"/* multi-line *\n"
+" * block comment */\n"
+"#define  4\n"
+"#define ccc  (5)",
+format("#define a 3\n"
+   "/* multi-line *\n"
+   " * block comment */\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style));
+
+  EXPECT_EQ("#define a3\n"
+"// multi-line line comment\n"
+"//\n"
+"#define  4\n"
+"#define ccc  (5)",
+format("#define a  3\n"
+   "// multi-line line comment\n"
+   "//\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style));
+
+  EXPECT_EQ("#define a 3\n"
+"// empty lines still break.\n"
+"\n"
+"#define  4\n"
+"#define ccc  (5)",
+format("#define a 3\n"
+   "// empty lines still break.\n"
+   "\n"
+   "#define  4\n"
+   "#define ccc  (5)",
+   Style));
+
+  // Test across empty lines
+  Style

[PATCH] D93591: [MLIR][SPIRV] Add (de-)serialization support for SpecConstantOpeation.

[Lei Zhang via Phabricator via cfe-commits]

antiagainst accepted this revision.
antiagainst added a comment.

Awesome, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93591

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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

This LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

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

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

[Florian Hahn via Phabricator via cfe-commits]

fhahn added reviewers: rjmccall, aaron.ballman, erichkeane.
fhahn added a comment.

Thanks for putting up the patch!

I think the code here and  D94366  could be 
simplified if we would introduce 2 codegen options to distinguish the 
differences between C and C++:

- `functions-must-progress=true/false`: C++ behavior >= c++11; all threads must 
make forward progress, we should be able to mark all functions as 
`mustprogress` and there should be no need to mark the individual loops as 
`mustprogress`.
- `loops-must-progress=all/none/c11`: `all` -> all loops are marked 
`mustprogress`, `none` -> no loops are marked, `c11` -> all loops that do not 
match the C11 escape hatch are marked `mustprogress`.

Now

- `c++11` and above, `functions-must-progress=true`, `loops-must-progress=none`,
- `c11` and above `functions-must-progress=false`, `loops-must-progress=c11`,
- `-ffinite-loops` -> `loops-must-progress=all`,
- `-fnofinite-loops` -> , `functions-must-progress=false`, 
`loops-must-progress=false`

In CodeGen, we always add `mustprogress` to functions exactly if 
`functions-must-progress=true`. For loops, we could have a helper 
`shouldMarkLoopAsMustProgress(bool C11Exception)`, which returns true depending 
on `loops-must-progress` and `C11Exceptions`, which the caller sets to true if 
the loop condition is one that is allowed for infinite loops in `C11`.

This should allow us to remove `CGF::FnIsMustProgress` and the code to update 
it; now either all functions have `mustprogress` or none have, depending on 
`functions-must-progress`. We still need `CGLoopInfo::MustProgress`, but we 
only need to update it depending on `loops-must-progress`,

I think this would overall simplify the handling in `Codegen`, but it's very 
possible that I am missing something from earlier discussions. I am also adding 
a few additional people who may have additional thought.

One things that's slightly odd with the proposal is that for C++, 
`loops-must-progress` would be `none`, which has the potential to be a bit 
confusing. We could make this clear in the documentation or just set it to 
`all`, although as I mentioned in the beginning, that should not really be 
necessary. It might be helpful/necessary if loops inlined from C++ function 
inside C functions should keep their `mustprogress` property.




Comment at: clang/docs/ClangCommandLineReference.rst:2290
+
+Assume that all loops terminate or never assume that they do. This takes 
precedence over the language standard.
+

It looks like the options are mostly ordered. Could you move it to the right 
place?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94367

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

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D94367#2489090 , @fhahn wrote:

> Thanks for putting up the patch!
>
> I think the code here and  D94366  could be 
> simplified if we would introduce 2 codegen options to distinguish the 
> differences between C and C++:
>
> - `functions-must-progress=true/false`: C++ behavior >= c++11; all threads 
> must make forward progress, we should be able to mark all functions as 
> `mustprogress` and there should be no need to mark the individual loops as 
> `mustprogress`.
> - `loops-must-progress=all/none/c11`: `all` -> all loops are marked 
> `mustprogress`, `none` -> no loops are marked, `c11` -> all loops that do not 
> match the C11 escape hatch are marked `mustprogress`.
>
> Now
>
> - `c++11` and above, `functions-must-progress=true`, 
> `loops-must-progress=none`,
> - `c11` and above `functions-must-progress=false`, `loops-must-progress=c11`,
> - `-ffinite-loops` -> `loops-must-progress=all`,
> - `-fnofinite-loops` -> , `functions-must-progress=false`, 
> `loops-must-progress=false`
>
> In CodeGen, we always add `mustprogress` to functions exactly if 
> `functions-must-progress=true`. For loops, we could have a helper 
> `shouldMarkLoopAsMustProgress(bool C11Exception)`, which returns true 
> depending on `loops-must-progress` and `C11Exceptions`, which the caller sets 
> to true if the loop condition is one that is allowed for infinite loops in 
> `C11`.
>
> This should allow us to remove `CGF::FnIsMustProgress` and the code to update 
> it; now either all functions have `mustprogress` or none have, depending on 
> `functions-must-progress`. We still need `CGLoopInfo::MustProgress`, but we 
> only need to update it depending on `loops-must-progress`,
>
> I think this would overall simplify the handling in `Codegen`, but it's very 
> possible that I am missing something from earlier discussions. I am also 
> adding a few additional people who may have additional thought.
>
> One things that's slightly odd with the proposal is that for C++, 
> `loops-must-progress` would be `none`, which has the potential to be a bit 
> confusing. We could make this clear in the documentation or just set it to 
> `all`, although as I mentioned in the beginning, that should not really be 
> necessary. It might be helpful/necessary if loops inlined from C++ function 
> inside C functions should keep their `mustprogress` property.

What happens when inlining `mustprogress` function into non-`mustprogress` 
function (or more generally, cross-language LTO)?
We'd need to ensure that in such cases the loops get annotated as 
`mustprogress`,
so they don't loose that status, which is why i think it may be better
to always annotate loops, even if their functions already are `mustprogress`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94367

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

[PATCH] D94367: [Clang][Driver] Add -ffinite-loops flags

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Just note: gcc enables -ffinite-loops with opt level -O2 and higher.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94367

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

[PATCH] D87147: PR-47391 : Two DIFile entries are describing the same file two different ways

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/CodeGenCXX/difile_entry.cpp:1
+// RUN: rm -rf %t/test_dir
+// RUN: mkdir -p %t/test_dir

@umesh.kalappa0 Can you clarify how this test demonstrates the behavior 
difference?

Reverting the code part does not make the test fail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87147

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

[PATCH] D87147: PR-47391 : Two DIFile entries are describing the same file two different ways

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/CodeGenCXX/difile_entry.cpp:1
+// RUN: rm -rf %t/test_dir
+// RUN: mkdir -p %t/test_dir

MaskRay wrote:
> @umesh.kalappa0 Can you clarify how this test demonstrates the behavior 
> difference?
> 
> Reverting the code part does not make the test fail.
Seems that absolute path needs to be used. `../test_dir/difile_entry.cpp` 
cannot demonstrate the difference. I'll improve the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87147

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

[clang] abfe348 - [test] Improve CodeGenCXX/difile_entry.cpp

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2021-01-10T12:24:49-08:00
New Revision: abfe348e6b4c50c750d70adcf0b99fd3d8d4132e

URL: 
https://github.com/llvm/llvm-project/commit/abfe348e6b4c50c750d70adcf0b99fd3d8d4132e
DIFF: 
https://github.com/llvm/llvm-project/commit/abfe348e6b4c50c750d70adcf0b99fd3d8d4132e.diff

LOG: [test] Improve CodeGenCXX/difile_entry.cpp

The test added in D87147 did not actually test PR47391.
Use an absolute path to test the canonicalization.

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGenCXX/difile_entry.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 8bc28b28c048..5bdda26a2e8e 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -409,6 +409,9 @@ llvm::DIFile *CGDebugInfo::getOrCreateFile(SourceLocation 
Loc) {
   FileID FID;
 
   if (Loc.isInvalid()) {
+// The DIFile used by the CU is distinct from the main source file. Call
+// createFile() below for canonicalization if the source file was specified
+// with an absolute path.
 FileName = TheCU->getFile()->getFilename();
   } else {
 PresumedLoc PLoc = SM.getPresumedLoc(Loc);

diff  --git a/clang/test/CodeGenCXX/difile_entry.cpp 
b/clang/test/CodeGenCXX/difile_entry.cpp
index 1ae36e4355ce..8bf6dc325470 100644
--- a/clang/test/CodeGenCXX/difile_entry.cpp
+++ b/clang/test/CodeGenCXX/difile_entry.cpp
@@ -1,12 +1,13 @@
-// RUN: rm -rf %t/test_dir
-// RUN: mkdir -p %t/test_dir
-// RUN: cd %t/test_dir
+/// PR47391: if the filename is absolute and starts with current working
+/// directory, there may be two ways describing the filename field of DIFile.
+/// Test that we canonicalize the DIFile.
+// RUN: rm -rf %t && mkdir %t && cd %t
 // RUN: cp %s .
-// RUN: %clang_cc1 -triple %itanium_abi_triple -main-file-name 
difile_entry.cpp -debug-info-kind=limited ../test_dir/difile_entry.cpp 
-std=c++11 -emit-llvm -o - | FileCheck  ../test_dir/difile_entry.cpp
+// RUN: %clang_cc1 -triple %itanium_abi_triple -main-file-name 
difile_entry.cpp -debug-info-kind=limited %t/difile_entry.cpp -std=c++11 
-emit-llvm -o - | FileCheck %s
 int x();
 static int i = x();
 
-// CHECK: [[FILE: *]] = !DIFile(filename: "{{.*}}difile_entry.cpp",
-// CHECK: {{.*}} = distinct !DISubprogram(name: "__cxx_global_var_init", 
scope: {{.*}}, file: [[FILE]]
-// CHECK: {{.*}} = distinct !DISubprogram(linkageName: 
"_GLOBAL__sub_I_difile_entry.cpp", scope: {{.*}}, file: [[FILE]]
-
+// CHECK: distinct !DIGlobalVariable(name: "i", {{.*}}, file: ![[#FILE:]],
+// CHECK: ![[#FILE]] = !DIFile(filename: "difile_entry.cpp", directory:
+// CHECK: distinct !DISubprogram(name: "__cxx_global_var_init", {{.*}}, file: 
![[#FILE]],
+// CHECK: distinct !DISubprogram(linkageName: 
"_GLOBAL__sub_I_difile_entry.cpp", {{.*}}, file: ![[#FILE]]



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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

Apart from my inline comment and the pre merge check this is superb. (I will 
not accept it, until we have reached a conclusion for the documentation.)




Comment at: clang/docs/ClangFormatStyleOptions.rst:338
+
+  int  = 12;
+  int b= 23;

This are now the examples from aligning the assignment. We now know that the 
rules for the alignments are the same, but someone new to it, just reading the 
documentation may be puzzled.

As @MyDeveloperDay said, most likely the script for generation has to be 
adapted?

The question here is now doing it now, or in a different change? It should 
happen before LLVM 12 will be released.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

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

[PATCH] D94359: [clangd] Remove ScratchFS from tests

[Adam Czachorowski via Phabricator via cfe-commits]

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

Thanks, that's better than my hacky workaround!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94359

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

[clang] 02bc320 - CGDebugInfo: Delete unused DIFile* parameter

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2021-01-10T15:03:40-08:00
New Revision: 02bc320545deb0212a43acae24fcf2383755d383

URL: 
https://github.com/llvm/llvm-project/commit/02bc320545deb0212a43acae24fcf2383755d383
DIFF: 
https://github.com/llvm/llvm-project/commit/02bc320545deb0212a43acae24fcf2383755d383.diff

LOG: CGDebugInfo: Delete unused DIFile* parameter

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 5bdda26a2e8e..74ec683fe6e5 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2404,7 +2404,7 @@ llvm::DIType *CGDebugInfo::CreateTypeDefinition(const 
RecordType *Ty) {
   // its members.  Finally, we create a descriptor for the complete type (which
   // may refer to the forward decl if the struct is recursive) and replace all
   // uses of the forward declaration with the final definition.
-  llvm::DICompositeType *FwdDecl = getOrCreateLimitedType(Ty, DefUnit);
+  llvm::DICompositeType *FwdDecl = getOrCreateLimitedType(Ty);
 
   const RecordDecl *D = RD->getDefinition();
   if (!D || !D->isCompleteDefinition())
@@ -3311,8 +3311,8 @@ llvm::DIType *CGDebugInfo::CreateTypeNode(QualType Ty, 
llvm::DIFile *Unit) {
   llvm_unreachable("type should have been unwrapped!");
 }
 
-llvm::DICompositeType *CGDebugInfo::getOrCreateLimitedType(const RecordType 
*Ty,
-   llvm::DIFile *Unit) 
{
+llvm::DICompositeType *
+CGDebugInfo::getOrCreateLimitedType(const RecordType *Ty) {
   QualType QTy(Ty, 0);
 
   auto *T = cast_or_null(getTypeOrNull(QTy));

diff  --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 59fe7929ef88..31fdd6b8ed18 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -603,8 +603,7 @@ class CGDebugInfo {
 
   /// Get the type from the cache or create a new partial type if
   /// necessary.
-  llvm::DICompositeType *getOrCreateLimitedType(const RecordType *Ty,
-llvm::DIFile *F);
+  llvm::DICompositeType *getOrCreateLimitedType(const RecordType *Ty);
 
   /// Create type metadata for a source language type.
   llvm::DIType *CreateTypeNode(QualType Ty, llvm::DIFile *Fg);



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

[PATCH] D93986: [clang-format] Add the possibility to align assignments spanning empty lines or comments

[Lukas Barth via Phabricator via cfe-commits]

tinloaf added a comment.

In D93986#2489140 , 
@HazardyKnusperkeks wrote:

> Apart from my inline comment and the pre merge check this is superb. (I will 
> not accept it, until we have reached a conclusion for the documentation.)

Regarding the pre-merge check: it is `clang-format` which failed. My local 
`clang-format` claims that the code is formatted according to the project's 
`.clang-format` (`git-clang-format` tells me that there are no modifications), 
but the `clang-format` on the server seems to disagree. I can of course just 
apply the supplied patch, but I would like to fix this so that it won't take me 
two tries in the future. Which version of `clang-format` is running on the 
build server? I'm running 12.0.0.




Comment at: clang/docs/ClangFormatStyleOptions.rst:338
+
+  int  = 12;
+  int b= 23;

HazardyKnusperkeks wrote:
> This are now the examples from aligning the assignment. We now know that the 
> rules for the alignments are the same, but someone new to it, just reading 
> the documentation may be puzzled.
> 
> As @MyDeveloperDay said, most likely the script for generation has to be 
> adapted?
> 
> The question here is now doing it now, or in a different change? It should 
> happen before LLVM 12 will be released.
I agree that this is not a good solution. I can probably adapt the doc 
generation script (have not yet looked at it in depth), but I currently have no 
good idea how to put the information into the source - i.e., where the dump 
script should get the info from. Here is an idea, which doesn't strike me as 
very elegant:

As far as I can tell, the script first takes the documentation block of the 
defined symbol (e.g., `AlignConsecutiveDeclarations`), and if the type of that 
symbol is an `enum`, then appends a list of possible enum values together with 
the documentation block of the respective enum value.

My suggestion would be to introduce some kind of tag into the documentation 
block of the defined symbol ( `DOC_IGNORE_ENUM` or something?), that, if 
present, deactivates this behavior and only takes the documentation of the 
defined symbol. I would then have to hand-code the different possible values 
and the respective examples into the documentation blocks of 
`AlignConsecutiveAssignments`, `AlignConsecutiveMacros`, 
`AlignConsecutiveBitFields` and `AlignConsecutiveDeclarations`. The 
documentation for `AlignConsecutiveStyle` would be ignored.

Would that be a feasible solution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93986

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

[PATCH] D93978: [clangd] DefineOutline doesn't require implementation file being saved

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

So I did some tweaking and I have a snapshot filesystem that will hold shared 
references to the contents, thereby avoiding the need to copy the file contents.
It works for rename and tweaks with no visible issues.
Reasoning behind this a long term goal of letting a user choose between using 
on disk or dirty buffers when building preambles. 
Obviously there, if using dirty buffers, we wouldn't want to be copying every 
open file (even unused files) during preamble building/checking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93978

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

[PATCH] D94380: [InstCombine] Update transformations to use poison for insertelement/shufflevector's placeholder value (1/2)

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added inline comments.



Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1728
   // Vec.
   if (IdxN % DstNumElts != 0 || IdxN + DstNumElts > VecNumElts) {
 replaceInstUsesWith(CI, UndefValue::get(CI.getType()));

Guarded by this branch condition, the for loop at line 1740 never adds a mask 
that makes `Shuffle` point to the second operand vector (which was previously 
Undef, but now Poison).



Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:2613
-  return InsertElementInst::Create(UndefValue::get(DestTy), Elem,
- 
Constant::getNullValue(Type::getInt32Ty(CI.getContext(;
 }

This creates `insertelement <1 x X86_mmx> undef, Elem, 0`
Replacing undef with poison has no visible effect.



Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1693
   Value *NewRHS = ConstOp1 ? NewC : V1;
-  return createBinOpShuffle(NewLHS, NewRHS, Mask);
+  return createBinOpShuffle(NewLHS, NewRHS, Mask, false);
 }

Be conservative, and create shufflevector with undef if Mask may point to the 
second vector


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94380

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

[PATCH] D94337: Add cuda header type for cuh files

[Justin Lebar via Phabricator via cfe-commits]

jlebar added a comment.

Hi, welcome!  Thank you for the careful and well-motivated first commit.  (I 
also see https://github.com/ccache/ccache/issues/772, hooray for noticing 
that...)

I am also not 100% sure how to write a test, but I think you may be able to 
observe the effect of your driver changes by running `clang -###`.  I'd expect 
you'd then see `-x cuda` in the output?

Same for the `.cuhi` change; I presume that with `-###` you'll be able to see 
that the driver is building a PCH with output name `.cuhi` even if that would 
crash if it actually went through.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94337

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

[PATCH] D94381: [test] Add Clang side tests for -fdebug-info-for-profiling

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: dblaikie, davidxl, wmi.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

There is currently a driver test but no test for its effect on linkageName & 
pass pipeline.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94381

Files:
  clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp


Index: clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp
@@ -0,0 +1,21 @@
+/// Normally -g1 does not add linkageName. -fdebug-info-for-profiling adds 
linkageName.
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple 
-debug-info-kind=line-tables-only %s -o - | FileCheck %s --check-prefix=LINE
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple 
-debug-info-kind=line-tables-only -fdebug-info-for-profiling %s -o - | 
FileCheck %s
+
+// LINE: = distinct !DISubprogram(name: "foo", scope:
+
+// CHECK: = distinct !DICompileUnit({{.*}}, debugInfoForProfiling: true,
+// CHECK: = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope:
+
+/// Add a DWARF discriminators pass for PGO.
+// RUN: %clang_cc1 -emit-llvm -fno-legacy-pass-manager -fdebug-pass-manager 
-O1 -fprofile-instrument-path=a.profdata %s -o - 2>&1 | FileCheck %s 
--check-prefix=NODISCR
+// RUN: %clang_cc1 -emit-llvm -fno-legacy-pass-manager -fdebug-pass-manager 
-O1 -fprofile-instrument-path=a.profdata -fdebug-info-for-profiling %s -o - 
2>&1 | FileCheck %s --check-prefix=DISCR
+
+// RUN: echo > %t.proftext
+// RUN: llvm-profdata merge %t.proftext -o %t.profdata
+// RUN: %clang_cc1 -emit-llvm -fno-legacy-pass-manager -fdebug-pass-manager 
-O1 -fprofile-instrument-use-path=%t.profdata -fdebug-info-for-profiling %s -o 
- 2>&1 | FileCheck %s --check-prefix=DISCR
+
+// NODISCR-NOT: Running pass: AddDiscriminatorsPass
+// DISCR:   Running pass: AddDiscriminatorsPass on {{.*}}
+
+void foo() {}


Index: clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp
@@ -0,0 +1,21 @@
+/// Normally -g1 does not add linkageName. -fdebug-info-for-profiling adds linkageName.
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -debug-info-kind=line-tables-only %s -o - | FileCheck %s --check-prefix=LINE
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -debug-info-kind=line-tables-only -fdebug-info-for-profiling %s -o - | FileCheck %s
+
+// LINE: = distinct !DISubprogram(name: "foo", scope:
+
+// CHECK: = distinct !DICompileUnit({{.*}}, debugInfoForProfiling: true,
+// CHECK: = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope:
+
+/// Add a DWARF discriminators pass for PGO.
+// RUN: %clang_cc1 -emit-llvm -fno-legacy-pass-manager -fdebug-pass-manager -O1 -fprofile-instrument-path=a.profdata %s -o - 2>&1 | FileCheck %s --check-prefix=NODISCR
+// RUN: %clang_cc1 -emit-llvm -fno-legacy-pass-manager -fdebug-pass-manager -O1 -fprofile-instrument-path=a.profdata -fdebug-info-for-profiling %s -o - 2>&1 | FileCheck %s --check-prefix=DISCR
+
+// RUN: echo > %t.proftext
+// RUN: llvm-profdata merge %t.proftext -o %t.profdata
+// RUN: %clang_cc1 -emit-llvm -fno-legacy-pass-manager -fdebug-pass-manager -O1 -fprofile-instrument-use-path=%t.profdata -fdebug-info-for-profiling %s -o - 2>&1 | FileCheck %s --check-prefix=DISCR
+
+// NODISCR-NOT: Running pass: AddDiscriminatorsPass
+// DISCR:   Running pass: AddDiscriminatorsPass on {{.*}}
+
+void foo() {}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94382: [clangd] Avoid recusion in TargetFinder::add()

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
nridge requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

Fixes https://github.com/clangd/clangd/issues/633


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D94382

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -787,6 +787,47 @@
"template  struct B");
 }
 
+TEST_F(TargetDeclTest, TypedefCascade) {
+  Code = R"cpp(
+struct C {
+  using type = int;
+};
+struct B {
+  using type = C::type;
+};
+struct A {
+  using type = B::type;
+};
+A::[[type]] waldo;
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc",
+   {"using type = int", Rel::Alias | Rel::Underlying},
+   {"using type = C::type", Rel::Alias | Rel::Underlying},
+   {"using type = B::type", Rel::Alias});
+}
+
+TEST_F(TargetDeclTest, RecursiveTemplate) {
+  Flags.push_back("-std=c++20"); // the test case uses concepts
+
+  Code = R"cpp(
+template 
+concept Leaf = false;
+
+template 
+struct descend_left {
+  using type = typename descend_left::[[type]];
+};
+
+template 
+struct descend_left {
+  using type = typename Tree::value;
+};
+  )cpp";
+  EXPECT_DECLS("DependentNameTypeLoc",
+   {"using type = typename descend_left::type",
+Rel::Alias});
+}
+
 TEST_F(TargetDeclTest, ObjC) {
   Flags = {"-xobjective-c"};
   Code = R"cpp(
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -330,6 +330,7 @@
   llvm::SmallDenseMap>
   Decls;
+  const Decl *CurrentlyProcessing = nullptr;
   RelSet Flags;
 
   template  void debug(T &Node, RelSet Flags) {
@@ -354,11 +355,28 @@
 return Result;
   }
 
+  bool seenDecl(const Decl *Dcl) const {
+if (CurrentlyProcessing == Dcl)
+  return true;
+for (const auto &D : Decls)
+  if (D.first == Dcl)
+return true;
+return false;
+  }
+
   void add(const Decl *Dcl, RelSet Flags) {
 const NamedDecl *D = llvm::dyn_cast_or_null(Dcl);
 if (!D)
   return;
 debug(*D, Flags);
+
+// Avoid recursion (which can arise in the presence of heuristic
+// resolution of dependent names) by exiting early if we have
+// already seen this decl.
+if (seenDecl(D))
+  return;
+CurrentlyProcessing = D;
+
 if (const UsingDirectiveDecl *UDD = llvm::dyn_cast(D))
   D = UDD->getNominatedNamespaceAsWritten();
 


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -787,6 +787,47 @@
"template  struct B");
 }
 
+TEST_F(TargetDeclTest, TypedefCascade) {
+  Code = R"cpp(
+struct C {
+  using type = int;
+};
+struct B {
+  using type = C::type;
+};
+struct A {
+  using type = B::type;
+};
+A::[[type]] waldo;
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc",
+   {"using type = int", Rel::Alias | Rel::Underlying},
+   {"using type = C::type", Rel::Alias | Rel::Underlying},
+   {"using type = B::type", Rel::Alias});
+}
+
+TEST_F(TargetDeclTest, RecursiveTemplate) {
+  Flags.push_back("-std=c++20"); // the test case uses concepts
+
+  Code = R"cpp(
+template 
+concept Leaf = false;
+
+template 
+struct descend_left {
+  using type = typename descend_left::[[type]];
+};
+
+template 
+struct descend_left {
+  using type = typename Tree::value;
+};
+  )cpp";
+  EXPECT_DECLS("DependentNameTypeLoc",
+   {"using type = typename descend_left::type",
+Rel::Alias});
+}
+
 TEST_F(TargetDeclTest, ObjC) {
   Flags = {"-xobjective-c"};
   Code = R"cpp(
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -330,6 +330,7 @@
   llvm::SmallDenseMap>
   Decls;
+  const Decl *CurrentlyProcessing = nullptr;
   RelSet Flags;
 
   template  void debug(T &Node, RelSet Flags) {
@@ -354,11 +355,28 @@
 return Result;
   }
 
+  bool seenDecl(const Decl *Dcl) const {

[PATCH] D94382: [clangd] Avoid recusion in TargetFinder::add()

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:790
 
+TEST_F(TargetDeclTest, TypedefCascade) {
+  Code = R"cpp(

This test case is not strictly related to the bug, but it adds test coverage 
for a scenario that I think is important, and that could be broken if we took a 
different approach to fixing this bug (see [my comments on the 
issue](https://github.com/clangd/clangd/issues/633#issuecomment-757586253) for 
details).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94382

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

[PATCH] D94382: [clangd] Avoid recursion in TargetFinder::add()

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:333
   Decls;
+  const Decl *CurrentlyProcessing = nullptr;
   RelSet Flags;

Whats the purpose in tracking this? Is the Decls lookup not enough?



Comment at: clang-tools-extra/clangd/FindTarget.cpp:361-364
+for (const auto &D : Decls)
+  if (D.first == Dcl)
+return true;
+return false;

Couldn't this just be a map lookup?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94382

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

[PATCH] D94382: [clangd] Avoid recursion in TargetFinder::add()

[Nathan Ridge via Phabricator via cfe-commits]

nridge marked an inline comment as done.
nridge added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:333
   Decls;
+  const Decl *CurrentlyProcessing = nullptr;
   RelSet Flags;

njames93 wrote:
> Whats the purpose in tracking this? Is the Decls lookup not enough?
No, because the possibly-recursive call is 
[here](https://searchfox.org/llvm/rev/9f2d9364b04c4d9651b1ec8612a3968b969fe71d/clang-tools-extra/clangd/FindTarget.cpp#366),
 but we only insert into `Decls` 
[here](https://searchfox.org/llvm/rev/9f2d9364b04c4d9651b1ec8612a3968b969fe71d/clang-tools-extra/clangd/FindTarget.cpp#417).



Comment at: clang-tools-extra/clangd/FindTarget.cpp:361-364
+for (const auto &D : Decls)
+  if (D.first == Dcl)
+return true;
+return false;

njames93 wrote:
> Couldn't this just be a map lookup?
Yup, good point!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94382

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

[PATCH] D94382: [clangd] Avoid recursion in TargetFinder::add()

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 315689.
nridge marked an inline comment as done.
nridge added a comment.

Use map lookup instead of iteration


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94382

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -787,6 +787,47 @@
"template  struct B");
 }
 
+TEST_F(TargetDeclTest, TypedefCascade) {
+  Code = R"cpp(
+struct C {
+  using type = int;
+};
+struct B {
+  using type = C::type;
+};
+struct A {
+  using type = B::type;
+};
+A::[[type]] waldo;
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc",
+   {"using type = int", Rel::Alias | Rel::Underlying},
+   {"using type = C::type", Rel::Alias | Rel::Underlying},
+   {"using type = B::type", Rel::Alias});
+}
+
+TEST_F(TargetDeclTest, RecursiveTemplate) {
+  Flags.push_back("-std=c++20"); // the test case uses concepts
+
+  Code = R"cpp(
+template 
+concept Leaf = false;
+
+template 
+struct descend_left {
+  using type = typename descend_left::[[type]];
+};
+
+template 
+struct descend_left {
+  using type = typename Tree::value;
+};
+  )cpp";
+  EXPECT_DECLS("DependentNameTypeLoc",
+   {"using type = typename descend_left::type",
+Rel::Alias});
+}
+
 TEST_F(TargetDeclTest, ObjC) {
   Flags = {"-xobjective-c"};
   Code = R"cpp(
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -330,6 +330,7 @@
   llvm::SmallDenseMap>
   Decls;
+  const Decl *CurrentlyProcessing = nullptr;
   RelSet Flags;
 
   template  void debug(T &Node, RelSet Flags) {
@@ -354,11 +355,23 @@
 return Result;
   }
 
+  bool seenDecl(const NamedDecl *D) const {
+return CurrentlyProcessing == D || Decls.count(D) != 0;
+  }
+
   void add(const Decl *Dcl, RelSet Flags) {
 const NamedDecl *D = llvm::dyn_cast_or_null(Dcl);
 if (!D)
   return;
 debug(*D, Flags);
+
+// Avoid recursion (which can arise in the presence of heuristic
+// resolution of dependent names) by exiting early if we have
+// already seen this decl.
+if (seenDecl(D))
+  return;
+CurrentlyProcessing = D;
+
 if (const UsingDirectiveDecl *UDD = llvm::dyn_cast(D))
   D = UDD->getNominatedNamespaceAsWritten();
 


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -787,6 +787,47 @@
"template  struct B");
 }
 
+TEST_F(TargetDeclTest, TypedefCascade) {
+  Code = R"cpp(
+struct C {
+  using type = int;
+};
+struct B {
+  using type = C::type;
+};
+struct A {
+  using type = B::type;
+};
+A::[[type]] waldo;
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc",
+   {"using type = int", Rel::Alias | Rel::Underlying},
+   {"using type = C::type", Rel::Alias | Rel::Underlying},
+   {"using type = B::type", Rel::Alias});
+}
+
+TEST_F(TargetDeclTest, RecursiveTemplate) {
+  Flags.push_back("-std=c++20"); // the test case uses concepts
+
+  Code = R"cpp(
+template 
+concept Leaf = false;
+
+template 
+struct descend_left {
+  using type = typename descend_left::[[type]];
+};
+
+template 
+struct descend_left {
+  using type = typename Tree::value;
+};
+  )cpp";
+  EXPECT_DECLS("DependentNameTypeLoc",
+   {"using type = typename descend_left::type",
+Rel::Alias});
+}
+
 TEST_F(TargetDeclTest, ObjC) {
   Flags = {"-xobjective-c"};
   Code = R"cpp(
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -330,6 +330,7 @@
   llvm::SmallDenseMap>
   Decls;
+  const Decl *CurrentlyProcessing = nullptr;
   RelSet Flags;
 
   template  void debug(T &Node, RelSet Flags) {
@@ -354,11 +355,23 @@
 return Result;
   }
 
+  bool seenDecl(const NamedDecl *D) const {
+return CurrentlyProcessing == D || Decls.count(D) != 0;
+  }
+
   void add(const Decl *Dcl, RelSet Flags) {
 const NamedDecl *D = llvm::dyn_ca

[PATCH] D93817: [InstCombine] Update transformations to use poison for insertelement/shufflevector's placeholder value (2/2)

[Juneyoung Lee via Phabricator via cfe-commits]

aqjune added inline comments.



Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:1221
   if (!isa(I->getOperand(1))) {
-I->setOperand(1, UndefValue::get(I->getOperand(1)->getType()));
 MadeChange = true;

This change is okay because it is splat (only the first vector of shufflevector 
was accessed)



Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1344
   }
 }
 // Create new operands for a shuffle that includes the constant of the

`Values` is the RHS of a new shufflevector below.
The poison elements are unused elems, so okay to be poison.



Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1441
 if (LR.second == nullptr)
-  LR.second = UndefValue::get(LR.first->getType());
+  LR.second = PoisonValue::get(LR.first->getType());
 return new ShuffleVectorInst(LR.first, LR.second, Mask);

When LR.second was nullptr, it was simply a placeholder (so okay to be poison)



Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:2355
 }
-return new ShuffleVectorInst(LHS, UndefValue::get(RHS->getType()), Elts);
+return new ShuffleVectorInst(LHS, PoisonValue::get(RHS->getType()), Elts);
   }

As seen in line 2353, Elts' indices are smaller than LHSWidth, so okay to be 
poison.



Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:551
+///%2  = shufflevector <2 x float> %1, poison, <0, 1, poison, poison>
+///%i2 = shufflevector <4 x float> %A, %2,<0, 1, 4, 5>
 static void replaceExtractElements(InsertElementInst *InsElt,

Copied from 
https://github.com/llvm/llvm-project/commit/ae945e7927e3a38cc3a44e829bc158c1ce5602ad



Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:602
   auto *WideVec =
-  new ShuffleVectorInst(ExtVecOp, UndefValue::get(ExtVecType), ExtendMask);
+  new ShuffleVectorInst(ExtVecOp, PoisonValue::get(ExtVecType), 
ExtendMask);
 

Okay to be poison (the added comment above replaceExtractElements has details)



Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1132
 
-  return new ShuffleVectorInst(FirstIE, UndefVec, Mask);
+  return new ShuffleVectorInst(FirstIE, PoisonVec, Mask);
 }

Since this is creating a splat, this shufflevector change and the insertelement 
change above is okay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93817

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

[clang] ffa6787 - [PowerPC] Add variants of 64-bit vector types for vec_sel.

[via cfe-commits]

Author: Esme-Yi
Date: 2021-01-11T03:52:16Z
New Revision: ffa67873a3f93a6baa0046221edd08a90b0db6f8

URL: 
https://github.com/llvm/llvm-project/commit/ffa67873a3f93a6baa0046221edd08a90b0db6f8
DIFF: 
https://github.com/llvm/llvm-project/commit/ffa67873a3f93a6baa0046221edd08a90b0db6f8.diff

LOG: [PowerPC] Add variants of 64-bit vector types for vec_sel.

Summary: This patch added variants of vec_sel and fixed bugzilla 46770.

Reviewed By: nemanjai

Differential Revision: https://reviews.llvm.org/D94162

Added: 


Modified: 
clang/lib/Headers/altivec.h
clang/test/CodeGen/builtins-ppc-vsx.c

Removed: 




diff  --git a/clang/lib/Headers/altivec.h b/clang/lib/Headers/altivec.h
index 2b82113de311..4d50d47d51b5 100644
--- a/clang/lib/Headers/altivec.h
+++ b/clang/lib/Headers/altivec.h
@@ -8281,6 +8281,46 @@ vec_sel(vector double __a, vector double __b, vector 
unsigned long long __c) {
((vector long long)__b & (vector long long)__c);
   return (vector double)__res;
 }
+
+static __inline__ vector bool long long __ATTRS_o_ai
+vec_sel(vector bool long long __a, vector bool long long __b,
+vector bool long long __c) {
+  return (__a & ~__c) | (__b & __c);
+}
+
+static __inline__ vector bool long long __ATTRS_o_ai
+vec_sel(vector bool long long __a, vector bool long long __b,
+vector unsigned long long __c) {
+  return (__a & ~(vector bool long long)__c) |
+ (__b & (vector bool long long)__c);
+}
+
+static __inline__ vector signed long long __ATTRS_o_ai
+vec_sel(vector signed long long __a, vector signed long long __b,
+vector bool long long __c) {
+  return (__a & ~(vector signed long long)__c) |
+ (__b & (vector signed long long)__c);
+}
+
+static __inline__ vector signed long long __ATTRS_o_ai
+vec_sel(vector signed long long __a, vector signed long long __b,
+vector unsigned long long __c) {
+  return (__a & ~(vector signed long long)__c) |
+ (__b & (vector signed long long)__c);
+}
+
+static __inline__ vector unsigned long long __ATTRS_o_ai
+vec_sel(vector unsigned long long __a, vector unsigned long long __b,
+vector bool long long __c) {
+  return (__a & ~(vector unsigned long long)__c) |
+ (__b & (vector unsigned long long)__c);
+}
+
+static __inline__ vector unsigned long long __ATTRS_o_ai
+vec_sel(vector unsigned long long __a, vector unsigned long long __b,
+vector unsigned long long __c) {
+  return (__a & ~__c) | (__b & __c);
+}
 #endif
 
 /* vec_vsel */

diff  --git a/clang/test/CodeGen/builtins-ppc-vsx.c 
b/clang/test/CodeGen/builtins-ppc-vsx.c
index cb9d484b5a77..bd0e66e69800 100644
--- a/clang/test/CodeGen/builtins-ppc-vsx.c
+++ b/clang/test/CodeGen/builtins-ppc-vsx.c
@@ -928,6 +928,66 @@ void test1() {
 // CHECK-LE: or <2 x i64>
 // CHECK-LE: bitcast <2 x i64> %{{[0-9]+}} to <2 x double>
 
+  res_vbll = vec_sel(vbll, vbll, vbll);
+// CHECK: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK: and <2 x i64> %{{[0-9]+}},
+// CHECK: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK: or <2 x i64>
+// CHECK-LE: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK-LE: and <2 x i64> %{{[0-9]+}},
+// CHECK-LE: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK-LE: or <2 x i64>
+
+  res_vbll = vec_sel(vbll, vbll, vull);
+// CHECK: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK: and <2 x i64> %{{[0-9]+}},
+// CHECK: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK: or <2 x i64>
+// CHECK-LE: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK-LE: and <2 x i64> %{{[0-9]+}},
+// CHECK-LE: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK-LE: or <2 x i64>
+
+  res_vsll = vec_sel(vsll, vsll, vbll);
+// CHECK: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK: and <2 x i64> %{{[0-9]+}},
+// CHECK: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK: or <2 x i64>
+// CHECK-LE: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK-LE: and <2 x i64> %{{[0-9]+}},
+// CHECK-LE: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK-LE: or <2 x i64>
+
+  res_vsll = vec_sel(vsll, vsll, vull);
+// CHECK: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK: and <2 x i64> %{{[0-9]+}},
+// CHECK: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK: or <2 x i64>
+// CHECK-LE: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK-LE: and <2 x i64> %{{[0-9]+}},
+// CHECK-LE: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK-LE: or <2 x i64>
+
+  res_vull = vec_sel(vull, vull, vbll);
+// CHECK: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK: and <2 x i64> %{{[0-9]+}},
+// CHECK: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK: or <2 x i64>
+// CHECK-LE: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK-LE: and <2 x i64> %{{[0-9]+}},
+// CHECK-LE: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK-LE: or <2 x i64>
+
+  res_vull = vec_sel(vull, vull, vull);
+// CHECK: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK: and <2 x i64> %{{[0-9]+}},
+// CHECK: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK: or <2 x i64>
+// CHECK-LE: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK-LE: and <2 x i64> %{{[

[PATCH] D94162: [PowerPC] Add variants of 64-bit vector types for vec_sel.

[EsmeYi 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 rGffa67873a3f9: [PowerPC] Add variants of 64-bit vector types 
for vec_sel. (authored by Esme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94162

Files:
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-vsx.c

Index: clang/test/CodeGen/builtins-ppc-vsx.c
===
--- clang/test/CodeGen/builtins-ppc-vsx.c
+++ clang/test/CodeGen/builtins-ppc-vsx.c
@@ -928,6 +928,66 @@
 // CHECK-LE: or <2 x i64>
 // CHECK-LE: bitcast <2 x i64> %{{[0-9]+}} to <2 x double>
 
+  res_vbll = vec_sel(vbll, vbll, vbll);
+// CHECK: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK: and <2 x i64> %{{[0-9]+}},
+// CHECK: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK: or <2 x i64>
+// CHECK-LE: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK-LE: and <2 x i64> %{{[0-9]+}},
+// CHECK-LE: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK-LE: or <2 x i64>
+
+  res_vbll = vec_sel(vbll, vbll, vull);
+// CHECK: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK: and <2 x i64> %{{[0-9]+}},
+// CHECK: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK: or <2 x i64>
+// CHECK-LE: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK-LE: and <2 x i64> %{{[0-9]+}},
+// CHECK-LE: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK-LE: or <2 x i64>
+
+  res_vsll = vec_sel(vsll, vsll, vbll);
+// CHECK: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK: and <2 x i64> %{{[0-9]+}},
+// CHECK: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK: or <2 x i64>
+// CHECK-LE: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK-LE: and <2 x i64> %{{[0-9]+}},
+// CHECK-LE: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK-LE: or <2 x i64>
+
+  res_vsll = vec_sel(vsll, vsll, vull);
+// CHECK: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK: and <2 x i64> %{{[0-9]+}},
+// CHECK: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK: or <2 x i64>
+// CHECK-LE: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK-LE: and <2 x i64> %{{[0-9]+}},
+// CHECK-LE: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK-LE: or <2 x i64>
+
+  res_vull = vec_sel(vull, vull, vbll);
+// CHECK: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK: and <2 x i64> %{{[0-9]+}},
+// CHECK: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK: or <2 x i64>
+// CHECK-LE: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK-LE: and <2 x i64> %{{[0-9]+}},
+// CHECK-LE: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK-LE: or <2 x i64>
+
+  res_vull = vec_sel(vull, vull, vull);
+// CHECK: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK: and <2 x i64> %{{[0-9]+}},
+// CHECK: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK: or <2 x i64>
+// CHECK-LE: xor <2 x i64> %{{[0-9]+}}, 
+// CHECK-LE: and <2 x i64> %{{[0-9]+}},
+// CHECK-LE: and <2 x i64> %{{[0-9]+}}, %{{[0-9]+}}
+// CHECK-LE: or <2 x i64>
+
   res_vf = vec_sqrt(vf);
 // CHECK: call <4 x float> @llvm.sqrt.v4f32(<4 x float> %{{[0-9]+}})
 // CHECK-LE: call <4 x float> @llvm.sqrt.v4f32(<4 x float> %{{[0-9]+}})
Index: clang/lib/Headers/altivec.h
===
--- clang/lib/Headers/altivec.h
+++ clang/lib/Headers/altivec.h
@@ -8281,6 +8281,46 @@
((vector long long)__b & (vector long long)__c);
   return (vector double)__res;
 }
+
+static __inline__ vector bool long long __ATTRS_o_ai
+vec_sel(vector bool long long __a, vector bool long long __b,
+vector bool long long __c) {
+  return (__a & ~__c) | (__b & __c);
+}
+
+static __inline__ vector bool long long __ATTRS_o_ai
+vec_sel(vector bool long long __a, vector bool long long __b,
+vector unsigned long long __c) {
+  return (__a & ~(vector bool long long)__c) |
+ (__b & (vector bool long long)__c);
+}
+
+static __inline__ vector signed long long __ATTRS_o_ai
+vec_sel(vector signed long long __a, vector signed long long __b,
+vector bool long long __c) {
+  return (__a & ~(vector signed long long)__c) |
+ (__b & (vector signed long long)__c);
+}
+
+static __inline__ vector signed long long __ATTRS_o_ai
+vec_sel(vector signed long long __a, vector signed long long __b,
+vector unsigned long long __c) {
+  return (__a & ~(vector signed long long)__c) |
+ (__b & (vector signed long long)__c);
+}
+
+static __inline__ vector unsigned long long __ATTRS_o_ai
+vec_sel(vector unsigned long long __a, vector unsigned long long __b,
+vector bool long long __c) {
+  return (__a & ~(vector unsigned long long)__c) |
+ (__b & (vector unsigned long long)__c);
+}
+
+static __inline__ vector unsigned long long __ATTRS_o_ai
+vec_sel(vector unsigned long long __a, vector unsigned long long __b,
+vector unsigned long long __c) {
+  return (__a & ~__c) | (__b & __c);
+}
 #endif
 
 /* vec_vsel */
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llv

[clang] 6215c1b - CGDebugInfo: Delete redundant test

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2021-01-10T22:22:06-08:00
New Revision: 6215c1b778f62433f3d79addc299a1bbd0e524d0

URL: 
https://github.com/llvm/llvm-project/commit/6215c1b778f62433f3d79addc299a1bbd0e524d0
DIFF: 
https://github.com/llvm/llvm-project/commit/6215c1b778f62433f3d79addc299a1bbd0e524d0.diff

LOG: CGDebugInfo: Delete redundant test

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 74ec683fe6e5..df8432d47bde 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3148,8 +3148,6 @@ llvm::DIType *CGDebugInfo::getTypeOrNull(QualType Ty) {
 
 void CGDebugInfo::completeTemplateDefinition(
 const ClassTemplateSpecializationDecl &SD) {
-  if (DebugKind <= codegenoptions::DebugLineTablesOnly)
-return;
   completeUnusedClass(SD);
 }
 



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

[clang] b8d2842 - CGDebugInfo: Delete unneeded UnwrapTypeForDebugInfo

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2021-01-10T22:22:07-08:00
New Revision: b8d28420885a42d16a57e02c28129d0eb92474a1

URL: 
https://github.com/llvm/llvm-project/commit/b8d28420885a42d16a57e02c28129d0eb92474a1
DIFF: 
https://github.com/llvm/llvm-project/commit/b8d28420885a42d16a57e02c28129d0eb92474a1.diff

LOG: CGDebugInfo: Delete unneeded UnwrapTypeForDebugInfo

Tested with stage 2 -DCMAKE_BUILD_TYPE=Debug clang, byte identical.

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index df8432d47bde..7dc6457d08d0 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3132,10 +3132,6 @@ static QualType UnwrapTypeForDebugInfo(QualType T, const 
ASTContext &C) {
 }
 
 llvm::DIType *CGDebugInfo::getTypeOrNull(QualType Ty) {
-
-  // Unwrap the type as needed for debug information.
-  Ty = UnwrapTypeForDebugInfo(Ty, CGM.getContext());
-
   auto It = TypeCache.find(Ty.getAsOpaquePtr());
   if (It != TypeCache.end()) {
 // Verify that the debug info still exists.



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

[PATCH] D94359: [clangd] Remove ScratchFS from tests

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: 
clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp:277
+  llvm::formatv(CDB, llvm::sys::path::convert_to_slash(testRoot()));
+  ;
   EXPECT_THAT(Command("x/foo.cc"), Contains("-DXYZZY"));

nit: extra semicolon


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94359

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

[PATCH] D94382: [clangd] Avoid recursion in TargetFinder::add()

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:379
 if (const TypedefNameDecl *TND = dyn_cast(D)) {
   add(TND->getUnderlyingType(), Flags | Rel::Underlying);
   Flags |= Rel::Alias; // continue with the alias.

rather than introducing a new member for `CurrentlyProcessing`, can't we just 
get away with checking the decl we are going to jump on here (and probably in 
namespace alias case) are the same (or maybe a parameter to add function 
indicating the parent/previous)?

it is definitely cleaner to have a member rather than checking individually but 
we have other types of ast nodes we can be currently processing (statements, 
nested namespecifiers etc. and they are not covered by this member. so it is a 
little bit confusing conceptually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94382

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

[PATCH] D94382: [clangd] Avoid recursion in TargetFinder::add()

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

oh and also thanks a lot for all the investigation and fix of the issue!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94382

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

[PATCH] D93591: [MLIR][SPIRV] Add (de-)serialization support for SpecConstantOpeation.

[Kareem Ergawy via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
ergawy marked an inline comment as done.
Closed by commit rGa40767ec8851: [MLIR][SPIRV] Add (de-)serialization support 
for SpecConstantOpeation. (authored by ergawy).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93591

Files:
  mlir/include/mlir/Dialect/SPIRV/IR/SPIRVBase.td
  mlir/lib/Dialect/SPIRV/IR/SPIRVOps.cpp
  mlir/lib/Target/SPIRV/Deserialization.cpp
  mlir/lib/Target/SPIRV/Serialization.cpp
  mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
  mlir/test/Target/SPIRV/spec-constant.mlir

Index: mlir/test/Target/SPIRV/spec-constant.mlir
===
--- mlir/test/Target/SPIRV/spec-constant.mlir
+++ mlir/test/Target/SPIRV/spec-constant.mlir
@@ -85,3 +85,34 @@
   // CHECK: spv.specConstantComposite @scc_vector (@sc_f32_1, @sc_f32_2, @sc_f32_3) : vector<3xf32>
   spv.specConstantComposite @scc_vector (@sc_f32_1, @sc_f32_2, @sc_f32_3) : vector<3 x f32>
 }
+
+// -
+
+spv.module Logical GLSL450 requires #spv.vce {
+
+  spv.specConstant @sc_i32_1 = 1 : i32
+
+  spv.func @use_composite() -> (i32) "None" {
+// CHECK: [[USE1:%.*]] = spv.mlir.referenceof @sc_i32_1 : i32
+// CHECK: [[USE2:%.*]] = spv.constant 0 : i32
+
+// CHECK: [[RES1:%.*]] = spv.SpecConstantOperation wraps "spv.ISub"([[USE1]], [[USE2]]) : (i32, i32) -> i32
+
+// CHECK: [[USE3:%.*]] = spv.mlir.referenceof @sc_i32_1 : i32
+// CHECK: [[USE4:%.*]] = spv.constant 0 : i32
+
+// CHECK: [[RES2:%.*]] = spv.SpecConstantOperation wraps "spv.ISub"([[USE3]], [[USE4]]) : (i32, i32) -> i32
+
+%0 = spv.mlir.referenceof @sc_i32_1 : i32
+%1 = spv.constant 0 : i32
+%2 = spv.SpecConstantOperation wraps "spv.ISub"(%0, %1) : (i32, i32) -> i32
+
+// CHECK: [[RES3:%.*]] = spv.SpecConstantOperation wraps "spv.IMul"([[RES1]], [[RES2]]) : (i32, i32) -> i32
+%3 = spv.SpecConstantOperation wraps "spv.IMul"(%2, %2) : (i32, i32) -> i32
+
+// Make sure deserialization continues from the right place after creating
+// the previous op.
+// CHECK: spv.ReturnValue [[RES3]]
+spv.ReturnValue %3 : i32
+  }
+}
Index: mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
===
--- mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
+++ mlir/test/Dialect/SPIRV/IR/structure-ops.mlir
@@ -780,6 +780,20 @@
 
 // -
 
+spv.module Logical GLSL450 {
+  spv.specConstant @sc = 42 : i32
+
+  spv.func @foo() -> i32 "None" {
+// CHECK: [[SC:%.*]] = spv.mlir.referenceof @sc
+%0 = spv.mlir.referenceof @sc : i32
+// CHECK: spv.SpecConstantOperation wraps "spv.ISub"([[SC]], [[SC]]) : (i32, i32) -> i32
+%1 = spv.SpecConstantOperation wraps "spv.ISub"(%0, %0) : (i32, i32) -> i32
+spv.ReturnValue %1 : i32
+  }
+}
+
+// -
+
 spv.module Logical GLSL450 {
   spv.func @foo() -> i32 "None" {
 %0 = spv.constant 1: i32
Index: mlir/lib/Target/SPIRV/Serialization.cpp
===
--- mlir/lib/Target/SPIRV/Serialization.cpp
+++ mlir/lib/Target/SPIRV/Serialization.cpp
@@ -204,6 +204,9 @@
   LogicalResult
   processSpecConstantCompositeOp(spirv::SpecConstantCompositeOp op);
 
+  LogicalResult
+  processSpecConstantOperationOp(spirv::SpecConstantOperationOp op);
+
   /// SPIR-V dialect supports OpUndef using spv.UndefOp that produces a SSA
   /// value to use with other operations. The SPIR-V spec recommends that
   /// OpUndef be generated at module level. The serialization generates an
@@ -711,6 +714,49 @@
   return processName(resultID, op.sym_name());
 }
 
+LogicalResult
+Serializer::processSpecConstantOperationOp(spirv::SpecConstantOperationOp op) {
+  uint32_t typeID = 0;
+  if (failed(processType(op.getLoc(), op.getType(), typeID))) {
+return failure();
+  }
+
+  auto resultID = getNextID();
+
+  SmallVector operands;
+  operands.push_back(typeID);
+  operands.push_back(resultID);
+
+  Block &block = op.getRegion().getBlocks().front();
+  Operation &enclosedOp = block.getOperations().front();
+
+  std::string enclosedOpName;
+  llvm::raw_string_ostream rss(enclosedOpName);
+  rss << "Op" << enclosedOp.getName().stripDialect();
+  auto enclosedOpcode = spirv::symbolizeOpcode(rss.str());
+
+  if (!enclosedOpcode) {
+op.emitError("Couldn't find op code for op ")
+<< enclosedOp.getName().getStringRef();
+return failure();
+  }
+
+  operands.push_back(static_cast(enclosedOpcode.getValue()));
+
+  // Append operands to the enclosed op to the list of operands.
+  for (Value operand : enclosedOp.getOperands()) {
+uint32_t id = getValueID(operand);
+assert(id && "use before def!");
+operands.push_back(id);
+  }
+
+  encodeInstructionInto(typesGlobalValues,
+spirv::Opcode::OpSpe

[PATCH] D94382: [clangd] Avoid recursion in TargetFinder::add()

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:379
 if (const TypedefNameDecl *TND = dyn_cast(D)) {
   add(TND->getUnderlyingType(), Flags | Rel::Underlying);
   Flags |= Rel::Alias; // continue with the alias.

kadircet wrote:
> rather than introducing a new member for `CurrentlyProcessing`, can't we just 
> get away with checking the decl we are going to jump on here (and probably in 
> namespace alias case) are the same (or maybe a parameter to add function 
> indicating the parent/previous)?
> 
> it is definitely cleaner to have a member rather than checking individually 
> but we have other types of ast nodes we can be currently processing 
> (statements, nested namespecifiers etc. and they are not covered by this 
> member. so it is a little bit confusing conceptually.
It takes a few steps to get back the recursive declaration:

 * we start with the original declaration `D`
 * we cast it to `TypedefNameDecl` here, call `getUnderlyingType()` and call 
`add(QualType)`
 * in `add(QualType)`, we construct a `TypeVisitor` and call `Visit()`
 * the visitor gets into `VisitDependentNameType()`, calls 
`getMembersReferencedViaDependentName()`, and that gets us back the same 
declaration `D` which we call `add(Decl)` on

So, we'd have to propagate the "previous decl" into `add(QualType)` and make it 
a member of the `TypeVisitor` at least, to be able to check there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94382

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

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

[Chirag Khandelwal via Phabricator via cfe-commits]

AMDChirag updated this revision to Diff 315708.
AMDChirag added a comment.

Added OMP delegating code for `createSection` (`EmitOMPSectionDirective`).
@fghanim working on clang lit test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91054

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp

Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -3546,6 +3546,64 @@
 }
 
 void CodeGenFunction::EmitOMPSectionsDirective(const OMPSectionsDirective &S) {
+  if (CGM.getLangOpts().OpenMPIRBuilder) {
+llvm::OpenMPIRBuilder &OMPBuilder = CGM.getOpenMPRuntime().getOMPBuilder();
+using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
+using BodyGenCallbackTy = llvm::OpenMPIRBuilder::StorableBodyGenCallbackTy;
+
+auto FiniCB = [this](InsertPointTy IP) {
+  OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
+};
+
+const CapturedStmt *ICS = S.getInnermostCapturedStmt();
+const Stmt *CapturedStmt = S.getInnermostCapturedStmt()->getCapturedStmt();
+const auto *CS = dyn_cast(CapturedStmt);
+llvm::SmallVector SectionCBVector;
+if (CS) {
+  for (const Stmt *SubStmt : CS->children()) {
+auto SectionCB = [this, SubStmt](InsertPointTy AllocaIP,
+ InsertPointTy CodeGenIP,
+ llvm::BasicBlock &FiniBB) {
+  OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP,
+ FiniBB);
+  OMPBuilderCBHelpers::EmitOMPRegionBody(*this, SubStmt, CodeGenIP,
+ FiniBB);
+};
+SectionCBVector.push_back(SectionCB);
+  }
+} else {
+  auto SectionCB = [this, CapturedStmt](InsertPointTy AllocaIP,
+InsertPointTy CodeGenIP,
+llvm::BasicBlock &FiniBB) {
+OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP, FiniBB);
+OMPBuilderCBHelpers::EmitOMPRegionBody(*this, CapturedStmt, CodeGenIP,
+   FiniBB);
+  };
+  SectionCBVector.push_back(SectionCB);
+}
+
+// Privatization callback that performs appropriate action for
+// shared/private/firstprivate/lastprivate/copyin/... variables.
+//
+// TODO: This defaults to shared right now.
+auto PrivCB = [](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+ llvm::Value &, llvm::Value &Val, llvm::Value *&ReplVal) {
+  // The next line is appropriate only for variables (Val) with the
+  // data-sharing attribute "shared".
+  ReplVal = &Val;
+
+  return CodeGenIP;
+};
+
+CGCapturedStmtInfo CGSI(*ICS, CR_OpenMP);
+CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(*this, &CGSI);
+llvm::OpenMPIRBuilder::InsertPointTy AllocaIP(
+AllocaInsertPt->getParent(), AllocaInsertPt->getIterator());
+Builder.restoreIP(OMPBuilder.createSections(
+Builder, AllocaIP, SectionCBVector, PrivCB, FiniCB, S.hasCancel(),
+S.getSingleClause()));
+return;
+  }
   {
 auto LPCRegion =
 CGOpenMPRuntime::LastprivateConditionalRAII::disable(*this, S);
@@ -3562,6 +3620,29 @@
 }
 
 void CodeGenFunction::EmitOMPSectionDirective(const OMPSectionDirective &S) {
+  if (CGM.getLangOpts().OpenMPIRBuilder) {
+llvm::OpenMPIRBuilder &OMPBuilder = CGM.getOpenMPRuntime().getOMPBuilder();
+using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
+
+const Stmt *SectionRegionBodyStmt = S.getAssociatedStmt();
+auto FiniCB = [this](InsertPointTy IP) {
+  OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
+};
+
+auto BodyGenCB = [SectionRegionBodyStmt, this](InsertPointTy AllocaIP,
+   InsertPointTy CodeGenIP,
+   llvm::BasicBlock &FiniBB) {
+  OMPBuilderCBHelpers::InlinedRegionBodyRAII IRB(*this, AllocaIP, FiniBB);
+  OMPBuilderCBHelpers::EmitOMPRegionBody(*this, SectionRegionBodyStmt,
+ CodeGenIP, FiniBB);
+};
+
+LexicalScope Scope(*this, S.getSourceRange());
+EmitStopPoint(&S);
+Builder.restoreIP(OMPBuilder.createSection(Builder, BodyGenCB, FiniCB));
+
+return;
+  }
   LexicalScope Scope(*this, S.getSourceRange());
   EmitStopPoint(&S);
   EmitStmt(S.getAssociatedStmt());
@@ -5970,7 +6051,9 @@
 llvm::OpenMPIRBuilder &OMPBuilder = CGM.getOpenMPRuntime().getOMPBuilder();
 // TODO: This check is necessary as we only generate `omp parallel` through
 // the OpenMPIRBuilder for now.
-if (S.getCancelRegion() == OMPD_parallel) {
+if (S.getCancelRegion() == OMPD_parallel ||
+  

[PATCH] D93822: [clang][Sema] Add diagnostics for implicit widening of multiplication result

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Ping

In D93822#2474475 , @dblaikie wrote:

> In D93822#2474355 , @lebedev.ri 
> wrote:
>
>> Thank you for taking a look!
>>
>> In D93822#2474236 , @dblaikie wrote:
>>
>>> I would guess it doesn't meet the on-by-default bar we would prefer for 
>>> Clang,
>>
>> Yeah, based on my initial observations, it's pretty chatty.
>>
>>> but might still be OK.
>>
>> I'm open to input here, but it would be good to have this at least in 
>> `-Wextra` (`-Weverything` always being a last resort option).
>
> Yeah, not sure how @rsmith, @rtrieu, or @aaron.ballman feel about what the 
> bar for warnings is these days - used to be a bit more hardcore about the "if 
> it can't be on by default it shouldn't be in clang" than we are these days, I 
> think. I'd guess -Wextra might be suitable.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93822

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

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Thanks for working on this!

I haven't looked at the whole patch in detail, but I looked at some parts 
(mainly the `SymbolIndex` API and what in-memory structures we store to 
implement it) and wrote a few thoughts.




Comment at: clang-tools-extra/clangd/XRefs.cpp:1843
+auto Kind = Callee.SymInfo.Kind;
+if (Kind != SK::Function && Kind != SK::InstanceMethod &&
+Kind != SK::ClassMethod && Kind != SK::StaticMethod &&

If memory usage of `Dex::RevRefs` becomes a concern, we could consider only 
storing the references that would pass this filter in the first place. That 
would trade off time spent building the reverse lookup (we'd have to do symbol 
lookups or keep around extra state to track the symbol kinds) for space savings.



Comment at: clang-tools-extra/clangd/index/Index.h:85
 
+struct RefersToResult {
+  /// The source location where the symbol is named.

As the protocol wants the outgoing calls grouped by symbol, we could consider 
storing them (and having the `SymbolIndex` API expose them) in that way.

The main motivation would be space savings on the storage side 
(`Dex::RevRefs`), as in the case of multiple calls to the same function we only 
need to store the target `SymbolID` once.

However, it may not be worth doing this, as having a large number of outgoing 
calls to the same target inside a given function is likely to be rare, and 
vectors have their own overhead. (It's also not clear to what extent the 
details of the LSP protocol should dictate the shape of the `SymbolIndex` API.)



Comment at: clang-tools-extra/clangd/index/Index.h:133
+  virtual bool
+  refersTo(const RefsRequest &Req,
+   llvm::function_ref Callback) const = 
0;

Perhaps we should have a distinct request structure, for future extensibility?



Comment at: clang-tools-extra/clangd/index/MemIndex.cpp:97
+  Req.Limit.getValueOr(std::numeric_limits::max());
+  for (const auto &Pair : Refs) {
+for (const auto &R : Pair.second) {

Looping over all refs seems expensive even for `MemIndex`. Perhaps we should 
have a reverse lookup table similar to `RevRefs` here as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93829

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