[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 338431.
ychen added a comment.

Fix comment. Ready for review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100739

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  llvm/docs/Coroutines.rst
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/Transforms/Coroutines/CoroFrame.cpp
  llvm/lib/Transforms/Coroutines/CoroInstr.h
  llvm/lib/Transforms/Coroutines/CoroInternal.h
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/lib/Transforms/Coroutines/Coroutines.cpp
  llvm/test/Transforms/Coroutines/ArgAddr.ll
  llvm/test/Transforms/Coroutines/coro-alloc-with-param-O0.ll
  llvm/test/Transforms/Coroutines/coro-alloca-01.ll

Index: llvm/test/Transforms/Coroutines/coro-alloca-01.ll
===
--- llvm/test/Transforms/Coroutines/coro-alloca-01.ll
+++ llvm/test/Transforms/Coroutines/coro-alloca-01.ll
@@ -8,7 +8,7 @@
   %x = alloca i64
   %y = alloca i64
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
-  %size = call i32 @llvm.coro.size.i32()
+  %size = call i32 @llvm.coro.size.i32(i1 false)
   %alloc = call i8* @malloc(i32 %size)
   %hdl = call i8* @llvm.coro.begin(token %id, i8* %alloc)
   br i1 %n, label %flag_true, label %flag_false
Index: llvm/test/Transforms/Coroutines/coro-alloc-with-param-O0.ll
===
--- llvm/test/Transforms/Coroutines/coro-alloc-with-param-O0.ll
+++ llvm/test/Transforms/Coroutines/coro-alloc-with-param-O0.ll
@@ -9,8 +9,8 @@
   %this.addr = alloca i64
   store i64 %this_arg, i64* %this.addr
   %this = load i64, i64* %this.addr
-  %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
-  %size = call i32 @llvm.coro.size.i32()
+  %id = call token @llvm.coro.id(i32 16, i8* null, i8* null, i8* null)
+  %size = call i32 @llvm.coro.size.i32(i1 true)
   %alloc = call i8* @myAlloc(i64 %this, i32 %size)
   %hdl = call i8* @llvm.coro.begin(token %id, i8* %alloc)
   %0 = call i8 @llvm.coro.suspend(token none, i1 false)
@@ -45,7 +45,7 @@
 ; CHECK: ret void
 
 declare i8* @llvm.coro.free(token, i8*)
-declare i32 @llvm.coro.size.i32()
+declare i32 @llvm.coro.size.i32(i1)
 declare i8  @llvm.coro.suspend(token, i1)
 declare void @llvm.coro.resume(i8*)
 declare void @llvm.coro.destroy(i8*)
Index: llvm/test/Transforms/Coroutines/ArgAddr.ll
===
--- llvm/test/Transforms/Coroutines/ArgAddr.ll
+++ llvm/test/Transforms/Coroutines/ArgAddr.ll
@@ -21,10 +21,10 @@
 ; CHECK-NEXT:store i32 [[TMP2]], i32* [[TMP1]], align 4
 ;
 entry:
-  %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null);
+  %id = call token @llvm.coro.id(i32 16, i8* null, i8* null, i8* null);
   %n.addr = alloca i32
   store i32 %n, i32* %n.addr ; this needs to go after coro.begin
-  %0 = tail call i32 @llvm.coro.size.i32()
+  %0 = tail call i32 @llvm.coro.size.i32(i1 true)
   %call = tail call i8* @malloc(i32 %0)
   %1 = tail call noalias nonnull i8* @llvm.coro.begin(token %id, i8* %call)
   %2 = bitcast i32* %n.addr to i8*
@@ -69,7 +69,7 @@
 declare void @ctor(i8* nocapture readonly)
 
 declare token @llvm.coro.id(i32, i8*, i8*, i8*)
-declare i32 @llvm.coro.size.i32()
+declare i32 @llvm.coro.size.i32(i1)
 declare i8* @llvm.coro.begin(token, i8*)
 declare i8 @llvm.coro.suspend(token, i1)
 declare i8* @llvm.coro.free(token, i8*)
Index: llvm/lib/Transforms/Coroutines/Coroutines.cpp
===
--- llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -234,6 +234,7 @@
   Shape.CoroBegin = nullptr;
   Shape.CoroEnds.clear();
   Shape.CoroSizes.clear();
+  Shape.CoroAligns.clear();
   Shape.CoroSuspends.clear();
 
   Shape.FrameTy = nullptr;
@@ -267,6 +268,10 @@
 continue;
   case Intrinsic::coro_size:
 CoroSizes.push_back(cast(II));
+HandleOverAlign = HandleOverAlign || CoroSizes.back()->isAlloc();
+break;
+  case Intrinsic::coro_align:
+CoroAligns.push_back(cast(II));
 break;
   case Intrinsic::coro_frame:
 CoroFrames.push_back(cast(II));
Index: llvm/lib/Transforms/Coroutines/CoroSplit.cpp
===
--- llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -997,23 +997,39 @@
   Shape.AsyncLowering.AsyncFuncPointer->setInitializer(NewFuncPtrStruct);
 }
 
-static void replaceFrameSize(coro::Shape ) {
+static void replaceFrameSizeAndAlign(coro::Shape ) {
   if (Shape.ABI == coro::ABI::Async)
 updateAsyncFuncPointerContextSize(Shape);
 
-  if (Shape.CoroSizes.empty())
-return;
+  if (!Shape.CoroSizes.empty()) {
+// In the 

[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 338430.
ychen added a comment.

fix comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100739

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  llvm/docs/Coroutines.rst
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/Transforms/Coroutines/CoroFrame.cpp
  llvm/lib/Transforms/Coroutines/CoroInstr.h
  llvm/lib/Transforms/Coroutines/CoroInternal.h
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/lib/Transforms/Coroutines/Coroutines.cpp
  llvm/test/Transforms/Coroutines/ArgAddr.ll
  llvm/test/Transforms/Coroutines/coro-alloc-with-param-O0.ll
  llvm/test/Transforms/Coroutines/coro-alloca-01.ll

Index: llvm/test/Transforms/Coroutines/coro-alloca-01.ll
===
--- llvm/test/Transforms/Coroutines/coro-alloca-01.ll
+++ llvm/test/Transforms/Coroutines/coro-alloca-01.ll
@@ -8,7 +8,7 @@
   %x = alloca i64
   %y = alloca i64
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
-  %size = call i32 @llvm.coro.size.i32()
+  %size = call i32 @llvm.coro.size.i32(i1 false)
   %alloc = call i8* @malloc(i32 %size)
   %hdl = call i8* @llvm.coro.begin(token %id, i8* %alloc)
   br i1 %n, label %flag_true, label %flag_false
Index: llvm/test/Transforms/Coroutines/coro-alloc-with-param-O0.ll
===
--- llvm/test/Transforms/Coroutines/coro-alloc-with-param-O0.ll
+++ llvm/test/Transforms/Coroutines/coro-alloc-with-param-O0.ll
@@ -9,8 +9,8 @@
   %this.addr = alloca i64
   store i64 %this_arg, i64* %this.addr
   %this = load i64, i64* %this.addr
-  %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
-  %size = call i32 @llvm.coro.size.i32()
+  %id = call token @llvm.coro.id(i32 16, i8* null, i8* null, i8* null)
+  %size = call i32 @llvm.coro.size.i32(i1 true)
   %alloc = call i8* @myAlloc(i64 %this, i32 %size)
   %hdl = call i8* @llvm.coro.begin(token %id, i8* %alloc)
   %0 = call i8 @llvm.coro.suspend(token none, i1 false)
@@ -45,7 +45,7 @@
 ; CHECK: ret void
 
 declare i8* @llvm.coro.free(token, i8*)
-declare i32 @llvm.coro.size.i32()
+declare i32 @llvm.coro.size.i32(i1)
 declare i8  @llvm.coro.suspend(token, i1)
 declare void @llvm.coro.resume(i8*)
 declare void @llvm.coro.destroy(i8*)
Index: llvm/test/Transforms/Coroutines/ArgAddr.ll
===
--- llvm/test/Transforms/Coroutines/ArgAddr.ll
+++ llvm/test/Transforms/Coroutines/ArgAddr.ll
@@ -21,10 +21,10 @@
 ; CHECK-NEXT:store i32 [[TMP2]], i32* [[TMP1]], align 4
 ;
 entry:
-  %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null);
+  %id = call token @llvm.coro.id(i32 16, i8* null, i8* null, i8* null);
   %n.addr = alloca i32
   store i32 %n, i32* %n.addr ; this needs to go after coro.begin
-  %0 = tail call i32 @llvm.coro.size.i32()
+  %0 = tail call i32 @llvm.coro.size.i32(i1 true)
   %call = tail call i8* @malloc(i32 %0)
   %1 = tail call noalias nonnull i8* @llvm.coro.begin(token %id, i8* %call)
   %2 = bitcast i32* %n.addr to i8*
@@ -69,7 +69,7 @@
 declare void @ctor(i8* nocapture readonly)
 
 declare token @llvm.coro.id(i32, i8*, i8*, i8*)
-declare i32 @llvm.coro.size.i32()
+declare i32 @llvm.coro.size.i32(i1)
 declare i8* @llvm.coro.begin(token, i8*)
 declare i8 @llvm.coro.suspend(token, i1)
 declare i8* @llvm.coro.free(token, i8*)
Index: llvm/lib/Transforms/Coroutines/Coroutines.cpp
===
--- llvm/lib/Transforms/Coroutines/Coroutines.cpp
+++ llvm/lib/Transforms/Coroutines/Coroutines.cpp
@@ -234,6 +234,7 @@
   Shape.CoroBegin = nullptr;
   Shape.CoroEnds.clear();
   Shape.CoroSizes.clear();
+  Shape.CoroAligns.clear();
   Shape.CoroSuspends.clear();
 
   Shape.FrameTy = nullptr;
@@ -267,6 +268,10 @@
 continue;
   case Intrinsic::coro_size:
 CoroSizes.push_back(cast(II));
+HandleOverAlign = HandleOverAlign || CoroSizes.back()->isAlloc();
+break;
+  case Intrinsic::coro_align:
+CoroAligns.push_back(cast(II));
 break;
   case Intrinsic::coro_frame:
 CoroFrames.push_back(cast(II));
Index: llvm/lib/Transforms/Coroutines/CoroSplit.cpp
===
--- llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -69,6 +69,7 @@
 #include 
 #include 
 #include 
+#include 
 
 using namespace llvm;
 
@@ -997,23 +998,39 @@
   Shape.AsyncLowering.AsyncFuncPointer->setInitializer(NewFuncPtrStruct);
 }
 
-static void replaceFrameSize(coro::Shape ) {
+static void replaceFrameSizeAndAlign(coro::Shape ) {
   if (Shape.ABI == coro::ABI::Async)
 updateAsyncFuncPointerContextSize(Shape);
 
-  if 

[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

[Yuanfang Chen via Phabricator via cfe-commits]

ychen created this revision.
ychen added reviewers: rjmccall, lxfind, ChuanqiXu.
Herald added a subscriber: hiraditya.
ychen requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, jdoerfert.
Herald added projects: clang, LLVM.

This is an alternative to D97915  which missed 
proper deallocation of the
over-allocated frame. This patch handles both allocations and deallocations.

Contrary to D97915 , this patch implements the 
over-allocation in the backend instead of the frontend since

- The alloca of the raw frame pointer (suppose we insert it in the frontend) 
would be included in the non-overaligned frame if we don't teach CoroFrame how 
to elide it.
- Only insert extra code when it is known that the frame is overaligned.
- Simpler implementation.
- Clients could turn it on/off by setting the newly introduced argument of 
`llvm.coro.size(i1 alloc)`.

`llvm.coro.size` gains it first argument `i1 alloc` to indicate users of the 
intrinsic are alloc/dealloc functions. It also indicates to LLVM that it should 
handle overaligned frames. One con of doing this is that many tests need fixup 
(if the general direction is agreed upon, I'll do that later).

It gets a little tricky finding the deallocation function. It is assumed that 
all CallInst users of `llvm.coro.free` are potentially deallocation functions. 
I think this should be the implicit assumption in practice although the 
documentation `Coroutines.rst` does not mention that. Happy to better idea in 
this regard.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100739

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  llvm/docs/Coroutines.rst
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/Transforms/Coroutines/CoroFrame.cpp
  llvm/lib/Transforms/Coroutines/CoroInstr.h
  llvm/lib/Transforms/Coroutines/CoroInternal.h
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/lib/Transforms/Coroutines/Coroutines.cpp
  llvm/test/Transforms/Coroutines/ArgAddr.ll
  llvm/test/Transforms/Coroutines/coro-alloc-with-param-O0.ll
  llvm/test/Transforms/Coroutines/coro-alloca-01.ll

Index: llvm/test/Transforms/Coroutines/coro-alloca-01.ll
===
--- llvm/test/Transforms/Coroutines/coro-alloca-01.ll
+++ llvm/test/Transforms/Coroutines/coro-alloca-01.ll
@@ -8,7 +8,7 @@
   %x = alloca i64
   %y = alloca i64
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
-  %size = call i32 @llvm.coro.size.i32()
+  %size = call i32 @llvm.coro.size.i32(i1 false)
   %alloc = call i8* @malloc(i32 %size)
   %hdl = call i8* @llvm.coro.begin(token %id, i8* %alloc)
   br i1 %n, label %flag_true, label %flag_false
Index: llvm/test/Transforms/Coroutines/coro-alloc-with-param-O0.ll
===
--- llvm/test/Transforms/Coroutines/coro-alloc-with-param-O0.ll
+++ llvm/test/Transforms/Coroutines/coro-alloc-with-param-O0.ll
@@ -9,8 +9,8 @@
   %this.addr = alloca i64
   store i64 %this_arg, i64* %this.addr
   %this = load i64, i64* %this.addr
-  %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
-  %size = call i32 @llvm.coro.size.i32()
+  %id = call token @llvm.coro.id(i32 16, i8* null, i8* null, i8* null)
+  %size = call i32 @llvm.coro.size.i32(i1 true)
   %alloc = call i8* @myAlloc(i64 %this, i32 %size)
   %hdl = call i8* @llvm.coro.begin(token %id, i8* %alloc)
   %0 = call i8 @llvm.coro.suspend(token none, i1 false)
@@ -45,7 +45,7 @@
 ; CHECK: ret void
 
 declare i8* @llvm.coro.free(token, i8*)
-declare i32 @llvm.coro.size.i32()
+declare i32 @llvm.coro.size.i32(i1)
 declare i8  @llvm.coro.suspend(token, i1)
 declare void @llvm.coro.resume(i8*)
 declare void @llvm.coro.destroy(i8*)
Index: llvm/test/Transforms/Coroutines/ArgAddr.ll
===
--- llvm/test/Transforms/Coroutines/ArgAddr.ll
+++ llvm/test/Transforms/Coroutines/ArgAddr.ll
@@ -21,10 +21,10 @@
 ; CHECK-NEXT:store i32 [[TMP2]], i32* [[TMP1]], align 4
 ;
 entry:
-  %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null);
+  %id = call token @llvm.coro.id(i32 16, i8* null, i8* null, i8* null);
   %n.addr = alloca i32
   store i32 %n, i32* %n.addr ; this needs to go after coro.begin
-  %0 = tail call i32 @llvm.coro.size.i32()
+  %0 = tail call i32 @llvm.coro.size.i32(i1 true)
   %call = tail call i8* @malloc(i32 %0)
   %1 = tail call noalias nonnull i8* @llvm.coro.begin(token %id, i8* %call)
   %2 = bitcast i32* %n.addr to i8*
@@ -69,7 +69,7 @@
 declare void @ctor(i8* nocapture readonly)
 
 declare token @llvm.coro.id(i32, i8*, i8*, i8*)
-declare i32 @llvm.coro.size.i32()
+declare i32 @llvm.coro.size.i32(i1)
 declare i8* @llvm.coro.begin(token, i8*)
 declare i8 @llvm.coro.suspend(token, i1)
 declare 

[PATCH] D100448: [RISCV][Clang] Add RVV AMO builtins

[Zakk Chen via Phabricator via cfe-commits]

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

LGTM. Please remove the ASM check in upstream patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100448

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

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[Michael Benfield via Phabricator via cfe-commits]

mbenfield updated this revision to Diff 338423.
mbenfield added a comment.

Fixed misfirings reported by aeubanks and added tests for those cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/Sema/vector-gcc-compat.c
  clang/test/Sema/warn-unused-but-set-parameters-cpp.cpp
  clang/test/Sema/warn-unused-but-set-parameters.c
  clang/test/Sema/warn-unused-but-set-variables-cpp.cpp
  clang/test/Sema/warn-unused-but-set-variables.c

Index: clang/test/Sema/warn-unused-but-set-variables.c
===
--- /dev/null
+++ clang/test/Sema/warn-unused-but-set-variables.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-variable -verify %s
+
+struct S {
+  int i;
+};
+
+int f0() {
+  int y; // expected-warning{{variable 'y' set but not used}}
+  y = 0;
+
+  int z __attribute__((unused));
+  z = 0;
+
+  struct S s; // expected-warning{{variable 's' set but not used}}
+  struct S t;
+  s = t;
+
+  int x;
+  x = 0;
+  return x;
+}
+
+void f1(void) {
+  (void)^() {
+int y; // expected-warning{{variable 'y' set but not used}}
+y = 0;
+
+int x;
+x = 0;
+return x;
+  };
+}
Index: clang/test/Sema/warn-unused-but-set-variables-cpp.cpp
===
--- /dev/null
+++ clang/test/Sema/warn-unused-but-set-variables-cpp.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-variable -verify %s
+
+struct S {
+  int i;
+};
+
+int f0() {
+  int y; // expected-warning{{variable 'y' set but not used}}
+  y = 0;
+
+  int z __attribute__((unused));
+  z = 0;
+
+  // In C++, don't warn for structs. (following gcc's behavior)
+  struct S s;
+  struct S t;
+  s = t;
+
+  int x;
+  x = 0;
+  return x + 5;
+}
+
+void f1(void) {
+  (void)^() {
+int y; // expected-warning{{variable 'y' set but not used}}
+y = 0;
+
+int x;
+x = 0;
+return x;
+  };
+}
Index: clang/test/Sema/warn-unused-but-set-parameters.c
===
--- /dev/null
+++ clang/test/Sema/warn-unused-but-set-parameters.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-parameter -verify %s
+
+int f0(int x,
+   int y, // expected-warning{{parameter 'y' set but not used}}
+   int z __attribute__((unused))) {
+  y = 0;
+  return x;
+}
+
+void f1(void) {
+  (void)^(int x,
+  int y, // expected-warning{{parameter 'y' set but not used}}
+  int z __attribute__((unused))) {
+y = 0;
+return x;
+  };
+}
+
+struct S {
+  int i;
+};
+
+void f3(struct S s) { // expected-warning{{parameter 's' set but not used}}
+  struct S t;
+  s = t;
+}
Index: clang/test/Sema/warn-unused-but-set-parameters-cpp.cpp
===
--- /dev/null
+++ clang/test/Sema/warn-unused-but-set-parameters-cpp.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -fblocks -fsyntax-only -Wunused-but-set-parameter -verify %s
+
+int f0(int x,
+   int y, // expected-warning{{parameter 'y' set but not used}}
+   int z __attribute__((unused))) {
+  y = 0;
+  return x;
+}
+
+void f1(void) {
+  (void)^(int x,
+  int y, // expected-warning{{parameter 'y' set but not used}}
+  int z __attribute__((unused))) {
+y = 0;
+return x;
+  };
+}
+
+struct S {
+  int i;
+};
+
+// In C++, don't warn for a struct (following gcc).
+void f3(struct S s) {
+  struct S t;
+  s = t;
+}
+
+// Make sure this doesn't warn.
+struct A {
+  int i;
+  A(int j) : i(j) {}
+};
Index: clang/test/Sema/vector-gcc-compat.c
===
--- clang/test/Sema/vector-gcc-compat.c
+++ clang/test/Sema/vector-gcc-compat.c
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 %s -verify -fsyntax-only -Weverything -triple x86_64-apple-darwin10
 
+#pragma clang diagnostic ignored "-Wunused-but-set-parameter"
+#pragma clang diagnostic ignored "-Wunused-but-set-variable"
+
 // Test the compatibility of clang's vector extensions with gcc's vector
 // extensions for C. Notably &&, ||, ?: and ! are not available.
 typedef long long v2i64 __attribute__((vector_size(16)));
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -434,7 +434,9 @@
   DiagnoseEmptyLoopBody(Elts[i], Elts[i + 1]);
   }
 
-  return CompoundStmt::Create(Context, Elts, L, R);
+  CompoundStmt *CS = CompoundStmt::Create(Context, Elts, L, R);
+  DiagnoseUnusedButSetVariables(CS);
+  return CS;
 }
 
 ExprResult
Index: clang/lib/Sema/SemaExpr.cpp

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D100567#2697415 , @yonghong-song 
wrote:

> To answer one of your questions above, if there is a function definition 
> before, dyn_cast(...) will return nullptr. I tried starting 
> from "Value" class and found dyn_cast to llvm::ConstantExpr will result in a 
> non-null object, but I did not trace down further with subclasses of 
> llvm::ConstantExpr.

Might be worth understanding what's going on here before committing this patch, 
as it might not be the right direction.

(perhaps the right thing to do is to look at the AST expression rather than 
whatever IR entity is created for it?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100567

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

[PATCH] D100581: [Clang] -Wunused-but-set-parameter and -Wunused-but-set-variable

[Michael Benfield via Phabricator via cfe-commits]

mbenfield added a comment.

In D100581#2693131 , @xbolva00 wrote:

>>> These warnings are not enabled by any other flags. This is different from 
>>> gcc, where -Wunused-but-set-variable is enabled by -Wextra in combination 
>>> with either -Wunused or -Wall.
>
> IMHO we should follow gcc here.

I'd be happy to do so, but there are two issues:

1. I'm not sure this is feasible and fits in with how Clang's diagnostics are 
organized. AFAICT clang's diagnostics are not set up to have a diagnostic 
enabled only if //two// other flags are set. If I'm wrong please let me know.

2. In gcc, this is how `-Wunused-parameter` behaves, but clang's 
`-Wunused-parameter` is already different. In clang, it's enabled by `-Wextra` 
regardless of `-Wall` or `-Wunused`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100581

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

[PATCH] D100448: [RISCV][Clang] Add RVV AMO builtins

[ShihPo Hung via Phabricator via cfe-commits]

arcbbb updated this revision to Diff 338419.
arcbbb added a comment.

re-formatted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100448

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoand.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamomax.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamomin.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoor.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoswap.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoxor.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vamoadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vamoand.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vamomax.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vamomin.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vamoor.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vamoswap.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vamoxor.c
  clang/utils/TableGen/RISCVVEmitter.cpp

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

[PATCH] D99158: [RISCV][WIP] Implement intrinsics for P extension

[Jim Lin via Phabricator via cfe-commits]

Jim added a comment.

In D99158#2695125 , @craig.topper 
wrote:

> Also can you please explain the vector codegen plan at a high level? Do you 
> intend to support auto vectorization or just using vector_size in C?

Currently, it just supports vector type operation (like v4i8+v4i8=>add8) in my 
local patch.




Comment at: llvm/test/CodeGen/RISCV/rvp/intrinsics-rv32p.ll:25
+  %1 = bitcast i32 %b.coerce to <4 x i8>
+  %2 = tail call <4 x i8> @llvm.riscv.add8.v4i8(<4 x i8> %0, <4 x i8> %1)
+  %3 = bitcast <4 x i8> %2 to i32

craig.topper wrote:
> I'm still not clear why we need to have two different ways to do the same 
> operation. Why isn't the C interface all scalar types or all vector types? 
> How do users choose which interface to use?
I will ask P extension intrinsic designer about your concern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99158

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

[PATCH] D100448: [RISCV][Clang] Add RVV AMO builtins

[ShihPo Hung via Phabricator via cfe-commits]

arcbbb updated this revision to Diff 338416.
arcbbb marked 3 inline comments as done.
arcbbb added a comment.

Addressed @khchen's comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100448

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoand.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamomax.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamomin.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoor.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoswap.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vamoxor.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vamoadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vamoand.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vamomax.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vamomin.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vamoor.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vamoswap.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vamoxor.c
  clang/utils/TableGen/RISCVVEmitter.cpp

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

[PATCH] D99158: [RISCV][WIP] Implement intrinsics for P extension

[Jim Lin via Phabricator via cfe-commits]

Jim updated this revision to Diff 338414.
Jim added a comment.

Fix typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99158

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/RISCV/rvp-intrinsics/rv32p.c
  clang/test/CodeGen/RISCV/rvp-intrinsics/rv64p.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVInstrInfoP.td
  llvm/test/CodeGen/RISCV/rvp/intrinsics-rv32p.ll
  llvm/test/CodeGen/RISCV/rvp/intrinsics-rv64p.ll

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

[PATCH] D100611: [RISCV] Add new attribute __clang_riscv_builtin_alias for intrinsics.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai added a comment.

In D100611#2697055 , @khchen wrote:

> Look good to me.
> BTW, we also need to update the  document 
>  
> later.

`AttributeReference.html` will be updated according to the content of 
`AttrDocs.td`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100611

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

[PATCH] D100611: [RISCV] Add new attribute __clang_riscv_builtin_alias for intrinsics.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai updated this revision to Diff 338411.
HsiangKai added a comment.

Update document.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100611

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/RISCV/riscv-attr-builtin-alias-err.c
  clang/test/CodeGen/RISCV/riscv-attr-builtin-alias.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -142,6 +142,7 @@
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PatchableFunctionEntry (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Pointer (SubjectMatchRule_record_not_is_union)
+// CHECK-NEXT: RISCVBuiltinAlias (SubjectMatchRule_function)
 // CHECK-NEXT: ReleaseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ReqdWorkGroupSize (SubjectMatchRule_function)
Index: clang/test/CodeGen/RISCV/riscv-attr-builtin-alias.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/riscv-attr-builtin-alias.c
@@ -0,0 +1,36 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv64 -emit-llvm -target-feature +experimental-v \
+// RUN:   %s -o - \
+// RUN:   | FileCheck %s
+
+#include 
+
+#define __rvv_generic \
+static inline __attribute__((__always_inline__, __nodebug__))
+
+__rvv_generic
+__attribute__((__clang_riscv_builtin_alias(__builtin_rvv_vadd_vv_i8m1)))
+vint8m1_t vadd_generic (vint8m1_t op0, vint8m1_t op1, size_t op2);
+
+// CHECK-LABEL: @test(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[OP0_ADDR:%.*]] = alloca , align 1
+// CHECK-NEXT:[[OP1_ADDR:%.*]] = alloca , align 1
+// CHECK-NEXT:[[VL_ADDR:%.*]] = alloca i64, align 8
+// CHECK-NEXT:[[RET:%.*]] = alloca , align 1
+// CHECK-NEXT:store  [[OP0:%.*]], * [[OP0_ADDR]], align 1
+// CHECK-NEXT:store  [[OP1:%.*]], * [[OP1_ADDR]], align 1
+// CHECK-NEXT:store i64 [[VL:%.*]], i64* [[VL_ADDR]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load , * [[OP0_ADDR]], align 1
+// CHECK-NEXT:[[TMP1:%.*]] = load , * [[OP1_ADDR]], align 1
+// CHECK-NEXT:[[TMP2:%.*]] = load i64, i64* [[VL_ADDR]], align 8
+// CHECK-NEXT:[[TMP3:%.*]] = call  @llvm.riscv.vadd.nxv8i8.nxv8i8.i64( [[TMP0]],  [[TMP1]], i64 [[TMP2]])
+// CHECK-NEXT:store  [[TMP3]], * [[RET]], align 1
+// CHECK-NEXT:[[TMP4:%.*]] = load , * [[RET]], align 1
+// CHECK-NEXT:ret  [[TMP4]]
+//
+vint8m1_t test(vint8m1_t op0, vint8m1_t op1, size_t vl) {
+  vint8m1_t ret = vadd_generic(op0, op1, vl);
+  return ret;
+}
Index: clang/test/CodeGen/RISCV/riscv-attr-builtin-alias-err.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/riscv-attr-builtin-alias-err.c
@@ -0,0 +1,19 @@
+// REQUIRES: riscv-registered-target
+// RUN: not %clang_cc1 -triple riscv64 -fsyntax-only -verify \
+// RUN:   -target-feature +experimental-v %s 2>&1 \
+// RUN: | FileCheck %s
+
+#include 
+
+#define __rvv_generic \
+static inline __attribute__((__always_inline__, __nodebug__))
+
+__rvv_generic
+__attribute__((__clang_riscv_builtin_alias(__builtin_rvv_vadd_vv_i8m1)))
+vint8m1_t vadd_generic (vint8m1_t op0, vint8m1_t op1, size_t op2);
+
+// CHECK: passing 'vint8m2_t' (aka '__rvv_int8m2_t') to parameter of incompatible type 'vint8m1_t'
+vint8m2_t test(vint8m2_t op0, vint8m2_t op1, size_t vl) {
+  vint8m2_t ret = vadd_generic(op0, op1, vl);
+  return ret;
+}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5120,6 +5120,7 @@
 #define GET_SVE_BUILTINS
 #define BUILTIN(name, types, attr) case SVE::BI##name:
 #include "clang/Basic/arm_sve_builtins.inc"
+#undef BUILTIN
 return true;
   }
 }
@@ -5146,6 +5147,37 @@
   D->addAttr(::new (S.Context) ArmBuiltinAliasAttr(S.Context, AL, Ident));
 }
 
+static bool RISCVVAliasValid(unsigned BuiltinID, StringRef AliasName) {
+  switch (BuiltinID) {
+  default:
+return false;
+#define BUILTIN(ID, TYPE, ATTRS) case RISCV::BI##ID:
+#include "clang/Basic/BuiltinsRISCV.def"
+#undef BUILTIN
+return true;
+  }
+}
+
+static void handleRISCVBuiltinAliasAttr(Sema , Decl *D,
+const ParsedAttr ) {
+  if (!AL.isArgIdent(0)) {
+S.Diag(AL.getLoc(), 

[PATCH] D100615: [RISCV][Driver] Make the ordering of CmdArgs consistent between RISCV::Linker and baremetal::Linker

[ShihPo Hung via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG27edaee84e3e: [RISCV][Driver] Make the ordering of CmdArgs 
consistent between RISCV::Linker… (authored by arcbbb).

Changed prior to commit:
  https://reviews.llvm.org/D100615?vs=338004=338410#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100615

Files:
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/test/Driver/riscv-args.c


Index: clang/test/Driver/riscv-args.c
===
--- /dev/null
+++ clang/test/Driver/riscv-args.c
@@ -0,0 +1,7 @@
+// Check the arguments are correctly passed
+
+// Make sure -T is the last with gcc-toolchain option
+// RUN: %clang -### -target riscv32 \
+// RUN:   --gcc-toolchain= -Xlinker --defsym=FOO=10 -T a.lds %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LD %s
+// CHECK-LD: {{.*}} "--defsym=FOO=10" {{.*}} "-T" "a.lds"
Index: clang/lib/Driver/ToolChains/RISCVToolchain.cpp
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -181,14 +181,14 @@
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtbegin)));
   }
 
+  AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
+
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
   Args.AddAllArgs(CmdArgs,
   {options::OPT_T_Group, options::OPT_e, options::OPT_s,
options::OPT_t, options::OPT_Z_Flag, options::OPT_r});
 
-  AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
-
   // TODO: add C++ includes and libs if compiling C++.
 
   if (!Args.hasArg(options::OPT_nostdlib) &&


Index: clang/test/Driver/riscv-args.c
===
--- /dev/null
+++ clang/test/Driver/riscv-args.c
@@ -0,0 +1,7 @@
+// Check the arguments are correctly passed
+
+// Make sure -T is the last with gcc-toolchain option
+// RUN: %clang -### -target riscv32 \
+// RUN:   --gcc-toolchain= -Xlinker --defsym=FOO=10 -T a.lds %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LD %s
+// CHECK-LD: {{.*}} "--defsym=FOO=10" {{.*}} "-T" "a.lds"
Index: clang/lib/Driver/ToolChains/RISCVToolchain.cpp
===
--- clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -181,14 +181,14 @@
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtbegin)));
   }
 
+  AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
+
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
   Args.AddAllArgs(CmdArgs,
   {options::OPT_T_Group, options::OPT_e, options::OPT_s,
options::OPT_t, options::OPT_Z_Flag, options::OPT_r});
 
-  AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
-
   // TODO: add C++ includes and libs if compiling C++.
 
   if (!Args.hasArg(options::OPT_nostdlib) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 27edaee - [RISCV][Driver] Make the ordering of CmdArgs consistent between RISCV::Linker and baremetal::Linker

[ShihPo Hung via cfe-commits]

Author: ShihPo Hung
Date: 2021-04-18T19:05:20-07:00
New Revision: 27edaee84e3ea4d160f742db0b4a04e236c4e26e

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

LOG: [RISCV][Driver] Make the ordering of CmdArgs consistent between 
RISCV::Linker and baremetal::Linker

In baremetal::Linker::ConstructJob, LinkerInput is handled prior to T_Group 
options,
but on the other side in RISCV::Linker::ConstructJob, it is opposite.

We want it to be consistent whether users are using RISCV::Linker or 
baremetal::Linker.

Reviewed By: MaskRay

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

Added: 
clang/test/Driver/riscv-args.c

Modified: 
clang/lib/Driver/ToolChains/RISCVToolchain.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp 
b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
index 9a29cc0985fc9..0b8c52096933d 100644
--- a/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
+++ b/clang/lib/Driver/ToolChains/RISCVToolchain.cpp
@@ -181,14 +181,14 @@ void RISCV::Linker::ConstructJob(Compilation , const 
JobAction ,
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crtbegin)));
   }
 
+  AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
+
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
   Args.AddAllArgs(CmdArgs,
   {options::OPT_T_Group, options::OPT_e, options::OPT_s,
options::OPT_t, options::OPT_Z_Flag, options::OPT_r});
 
-  AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
-
   // TODO: add C++ includes and libs if compiling C++.
 
   if (!Args.hasArg(options::OPT_nostdlib) &&

diff  --git a/clang/test/Driver/riscv-args.c b/clang/test/Driver/riscv-args.c
new file mode 100644
index 0..7b68df977f7c3
--- /dev/null
+++ b/clang/test/Driver/riscv-args.c
@@ -0,0 +1,7 @@
+// Check the arguments are correctly passed
+
+// Make sure -T is the last with gcc-toolchain option
+// RUN: %clang -### -target riscv32 \
+// RUN:   --gcc-toolchain= -Xlinker --defsym=FOO=10 -T a.lds %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LD %s
+// CHECK-LD: {{.*}} "--defsym=FOO=10" {{.*}} "-T" "a.lds"



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

[clang-tools-extra] 8969762 - [clangd][test] Fix build error of FeatureModulesTests

[via cfe-commits]

Author: Pan, Tao
Date: 2021-04-19T08:56:07+08:00
New Revision: 8969762fb1cf3b05adef5d6158b080548a9363e2

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

LOG: [clangd][test] Fix build error of FeatureModulesTests

clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp:33:58: error:
could not convert ‘(const char*)""’ from ‘const char*’ to
llvm::StringLiteral’
   llvm::StringLiteral kind() const override { return ""; };

Reviewed By: kadircet

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp 
b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
index d73b805499f7f..8611f16688c7f 100644
--- a/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
@@ -30,7 +30,9 @@ TEST(FeatureModulesTest, ContributesTweak) {
 return error("not implemented");
   }
   std::string title() const override { return id(); }
-  llvm::StringLiteral kind() const override { return ""; };
+  llvm::StringLiteral kind() const override {
+return llvm::StringLiteral("");
+  };
 };
 
 void contributeTweaks(std::vector> ) override {



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

[PATCH] D99031: [clang-format] Fix CompactNamespaces corner case when AllowShortLambdasOnASingleLine/BraceWrapping.BeforeLambdaBody are set

[Ahmed Mahdy via Phabricator via cfe-commits]

aybassiouny updated this revision to Diff 338403.
aybassiouny edited the summary of this revision.
aybassiouny added a comment.

After rechecking, turns out `AllowShortLambdasOnASingleLine` and 
`BeforeLambdaBody` both need to be turned on in order for the regression to be 
expressed, this affects the UT.

Also added `verifyFormat` check as suggested, it does not fail btw without this 
patch.


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

https://reviews.llvm.org/D99031

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2589,6 +2589,25 @@
Style));
 }
 
+TEST_F(FormatTest, FormatsCompactNamespacesLambdaRegression) {
+  // Make sure compact namespaces are not confused with lambdas
+  FormatStyle CompactNamespacesStyle{getLLVMStyle()};
+  CompactNamespacesStyle.CompactNamespaces = true;
+  CompactNamespacesStyle.AllowShortLambdasOnASingleLine = 
FormatStyle::SLS_None;
+  CompactNamespacesStyle.BreakBeforeBraces = FormatStyle::BS_Custom;
+  CompactNamespacesStyle.BraceWrapping.BeforeLambdaBody = true;
+  verifyFormat("namespace out { namespace in {\n"
+   "}} // namespace out::in",
+   CompactNamespacesStyle);
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   "} // namespace out",
+   CompactNamespacesStyle));
+}
+
 TEST_F(FormatTest, FormatsExternC) {
   verifyFormat("extern \"C\" {\nint a;");
   verifyFormat("extern \"C\" {}");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3494,7 +3494,8 @@
 }
 static bool isAllmanLambdaBrace(const FormatToken ) {
   return (Tok.is(tok::l_brace) && Tok.is(BK_Block) &&
-  !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral));
+  !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral) &&
+  !Tok.Previous->Previous->is(tok::kw_namespace));
 }
 
 static bool isAllmanBraceIncludedBreakableLambda(


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2589,6 +2589,25 @@
Style));
 }
 
+TEST_F(FormatTest, FormatsCompactNamespacesLambdaRegression) {
+  // Make sure compact namespaces are not confused with lambdas
+  FormatStyle CompactNamespacesStyle{getLLVMStyle()};
+  CompactNamespacesStyle.CompactNamespaces = true;
+  CompactNamespacesStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  CompactNamespacesStyle.BreakBeforeBraces = FormatStyle::BS_Custom;
+  CompactNamespacesStyle.BraceWrapping.BeforeLambdaBody = true;
+  verifyFormat("namespace out { namespace in {\n"
+   "}} // namespace out::in",
+   CompactNamespacesStyle);
+  EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+   "namespace in {\n"
+   "} // namespace in\n"
+   "} // namespace out",
+   CompactNamespacesStyle));
+}
+
 TEST_F(FormatTest, FormatsExternC) {
   verifyFormat("extern \"C\" {\nint a;");
   verifyFormat("extern \"C\" {}");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3494,7 +3494,8 @@
 }
 static bool isAllmanLambdaBrace(const FormatToken ) {
   return (Tok.is(tok::l_brace) && Tok.is(BK_Block) &&
-  !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral));
+  !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral) &&
+  !Tok.Previous->Previous->is(tok::kw_namespace));
 }
 
 static bool isAllmanBraceIncludedBreakableLambda(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100282: [Coroutines] Set presplit attribute in Clang instead of CoroEarly pass

[Xun Li via Phabricator via cfe-commits]

lxfind added a comment.

This broke MLIR tests.
It seems that MLIR tests depend on CoroEarly to be able to annotate coroutine 
function properly based on the intrinsics.
Given that, I am now convinced we shouldn't set the attribute in the frontend. 
Instead we should simply move CoroEarly to before AlwaysInliner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100282

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

[clang] 5faba87 - Revert "[Coroutines] Set presplit attribute in Clang instead of CoroEarly pass"

[Xun Li via cfe-commits]

Author: Xun Li
Date: 2021-04-18T17:22:28-07:00
New Revision: 5faba87938779c595f2b4e40f933bae6571bc421

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

LOG: Revert "[Coroutines] Set presplit attribute in Clang instead of CoroEarly 
pass"

This reverts commit fa6b54c44ab1d5f579304eadb7ac8bd7e72d0e77.
The commited patch broke mlir tests. It seems that mlir tests depend on 
coroutine function properties set in CoroEarly pass.

Added: 


Modified: 
clang/lib/CodeGen/CGCoroutine.cpp
clang/test/CodeGenCoroutines/coro-always-inline.cpp
llvm/lib/Transforms/Coroutines/CoroEarly.cpp
llvm/test/Transforms/Coroutines/coro-debug-O2.ll
llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
llvm/test/Transforms/Coroutines/coro-split-01.ll
llvm/test/Transforms/Coroutines/coro-split-recursive.ll
llvm/test/Transforms/Coroutines/ex0.ll
llvm/test/Transforms/Coroutines/ex1.ll
llvm/test/Transforms/Coroutines/ex2.ll
llvm/test/Transforms/Coroutines/ex3.ll
llvm/test/Transforms/Coroutines/ex4.ll
llvm/test/Transforms/Coroutines/ex5.ll
llvm/test/Transforms/Coroutines/phi-coro-end.ll
llvm/test/Transforms/Coroutines/restart-trigger.ll

Removed: 
clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp



diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index fcf8fe062367f..ca071d3d2e80f 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -558,8 +558,6 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt ) {
   CurCoro.Data->SuspendBB = RetBB;
   assert(ShouldEmitLifetimeMarkers &&
  "Must emit lifetime intrinsics for coroutines");
-  // CORO_PRESPLIT_ATTR = UNPREPARED_FOR_SPLIT
-  CurFn->addFnAttr("coroutine.presplit", "0");
 
   // Backend is allowed to elide memory allocations, to help it, emit
   // auto mem = coro.alloc() ? 0 : ... allocation code ...;

diff  --git a/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp 
b/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
deleted file mode 100644
index e4aa14a6ac397..0
--- a/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
+++ /dev/null
@@ -1,54 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -fexperimental-new-pass-manager -O0 %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -fexperimental-new-pass-manager -fno-inline -O0 %s -o - | FileCheck 
%s
-
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -O0 %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -fno-inline -O0 %s -o - | FileCheck %s
-
-namespace std {
-namespace experimental {
-
-struct handle {};
-
-struct awaitable {
-  bool await_ready() noexcept { return true; }
-  // CHECK-NOT: await_suspend
-  inline void __attribute__((__always_inline__)) await_suspend(handle) 
noexcept {}
-  bool await_resume() noexcept { return true; }
-};
-
-template 
-struct coroutine_handle {
-  static handle from_address(void *address) noexcept { return {}; }
-};
-
-template 
-struct coroutine_traits {
-  struct promise_type {
-awaitable initial_suspend() { return {}; }
-awaitable final_suspend() noexcept { return {}; }
-void return_void() {}
-T get_return_object() { return T(); }
-void unhandled_exception() {}
-  };
-};
-} // namespace experimental
-} // namespace std
-
-// CHECK-LABEL: @_Z3foov
-// CHECK-LABEL: entry:
-// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
-// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
-// CHECK: [[CAST0:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST0]])
-// CHECK: [[CAST1:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST1]])
-
-// CHECK: [[CAST2:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST2]])
-// CHECK: [[CAST3:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST3]])
-void foo() { co_return; }

diff  --git a/clang/test/CodeGenCoroutines/coro-always-inline.cpp 
b/clang/test/CodeGenCoroutines/coro-always-inline.cpp
index 6ba5a6f124169..e4aa14a6ac397 100644
--- 

[PATCH] D100733: [clang] NFC: change uses of `Expr->getValueKind` into `is?Value`

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D100733#2697537 , @aaronpuchert 
wrote:

> The change seems to be correct, but I'm wondering if `x.getValueKind() == 
> VK_*Value` doesn't have one advantage over `x.is*Value()`: it's obvious that 
> this is exclusive with the other values. Especially with `isRValue()` it 
> might not be so obvious, because Clang doesn't follow the C++11 terminology 
> with this.
>
> But it's admittedly shorter, so I'd be willing to approve this.

This came up in a patch where I am experimenting with a new value category.
The helpers 'help' a lot more when you are changing some of these tests from 
testing just one category, to testing a combination of categories (like 
GLValue).

But this is just the easy pickings of the patch, which is still WIP, and I may 
need in the future for some more drastic change to deal with the code that just 
stores the kind in a variable and tests that later.

It would be useful if you could do:

  VK = Expr->getValueKind();
  if (VK.isGLValue()) {

I may need to do something like that later.




Comment at: clang/lib/Sema/SemaExpr.cpp:5522
 }
 VK = LHSExp->getValueKind();
 if (VK != VK_RValue)

aaronpuchert wrote:
> There might be a certain benefit to using `LHSExp->getValueKind()` above when 
> we use it here again: that makes it more obvious what we're trying to achieve 
> in that `if`. (Namely changing the value category.)
Or just making the same kind of change here again:
```
if (!LHSExp->isRValue())
  OK = OK_VectorComponent;
VK = LHSExp->getValueKind();
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100733

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

[PATCH] D100733: [clang] NFC: change uses of `Expr->getValueKind` into `is?Value`

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

The change seems to be correct, but I'm wondering if `x.getValueKind() == 
VK_*Value` doesn't have one advantage over `x.is*Value()`: it's obvious that 
this is exclusive with the other values. Especially with `isRValue()` it might 
not be so obvious, because Clang doesn't follow the C++11 terminology with this.

But it's admittedly shorter, so I'd be willing to approve this.




Comment at: clang/lib/Sema/SemaExpr.cpp:5522
 }
 VK = LHSExp->getValueKind();
 if (VK != VK_RValue)

There might be a certain benefit to using `LHSExp->getValueKind()` above when 
we use it here again: that makes it more obvious what we're trying to achieve 
in that `if`. (Namely changing the value category.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100733

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

[clang] fa6b54c - [Coroutines] Set presplit attribute in Clang instead of CoroEarly pass

[Xun Li via cfe-commits]

Author: Xun Li
Date: 2021-04-18T15:41:09-07:00
New Revision: fa6b54c44ab1d5f579304eadb7ac8bd7e72d0e77

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

LOG: [Coroutines] Set presplit attribute in Clang instead of CoroEarly pass

Presplit coroutines cannot be inlined. During AlwaysInliner we check if a 
function is a presplit coroutine, if so we skip inlining.
The presplit coroutine attributes are set in CoroEarly pass.
However in O0 pipeline, AlwaysInliner runs before CoroEarly, so the attribute 
isn't set yet and will still inline the coroutine.
This causes Clang to crash: https://bugs.llvm.org/show_bug.cgi?id=49920

To fix this, we set the attributes in the Clang front-end instead of in 
CoroEarly pass.

Reviewed By: rjmccall, ChuanqiXu

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

Added: 
clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp

Modified: 
clang/lib/CodeGen/CGCoroutine.cpp
clang/test/CodeGenCoroutines/coro-always-inline.cpp
llvm/lib/Transforms/Coroutines/CoroEarly.cpp
llvm/test/Transforms/Coroutines/coro-debug-O2.ll
llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
llvm/test/Transforms/Coroutines/coro-split-01.ll
llvm/test/Transforms/Coroutines/coro-split-recursive.ll
llvm/test/Transforms/Coroutines/ex0.ll
llvm/test/Transforms/Coroutines/ex1.ll
llvm/test/Transforms/Coroutines/ex2.ll
llvm/test/Transforms/Coroutines/ex3.ll
llvm/test/Transforms/Coroutines/ex4.ll
llvm/test/Transforms/Coroutines/ex5.ll
llvm/test/Transforms/Coroutines/phi-coro-end.ll
llvm/test/Transforms/Coroutines/restart-trigger.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index ca071d3d2e80f..fcf8fe062367f 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -558,6 +558,8 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt ) {
   CurCoro.Data->SuspendBB = RetBB;
   assert(ShouldEmitLifetimeMarkers &&
  "Must emit lifetime intrinsics for coroutines");
+  // CORO_PRESPLIT_ATTR = UNPREPARED_FOR_SPLIT
+  CurFn->addFnAttr("coroutine.presplit", "0");
 
   // Backend is allowed to elide memory allocations, to help it, emit
   // auto mem = coro.alloc() ? 0 : ... allocation code ...;

diff  --git a/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp 
b/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
new file mode 100644
index 0..e4aa14a6ac397
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fexperimental-new-pass-manager -O0 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fexperimental-new-pass-manager -fno-inline -O0 %s -o - | FileCheck 
%s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -O0 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fno-inline -O0 %s -o - | FileCheck %s
+
+namespace std {
+namespace experimental {
+
+struct handle {};
+
+struct awaitable {
+  bool await_ready() noexcept { return true; }
+  // CHECK-NOT: await_suspend
+  inline void __attribute__((__always_inline__)) await_suspend(handle) 
noexcept {}
+  bool await_resume() noexcept { return true; }
+};
+
+template 
+struct coroutine_handle {
+  static handle from_address(void *address) noexcept { return {}; }
+};
+
+template 
+struct coroutine_traits {
+  struct promise_type {
+awaitable initial_suspend() { return {}; }
+awaitable final_suspend() noexcept { return {}; }
+void return_void() {}
+T get_return_object() { return T(); }
+void unhandled_exception() {}
+  };
+};
+} // namespace experimental
+} // namespace std
+
+// CHECK-LABEL: @_Z3foov
+// CHECK-LABEL: entry:
+// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
+// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
+// CHECK: [[CAST0:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST0]])
+// CHECK: [[CAST1:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST1]])
+
+// CHECK: [[CAST2:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST2]])
+// CHECK: [[CAST3:%[0-9]+]] = bitcast 

[clang] c0211e8 - Revert "[Coroutines] Move CoroEarly pass to before AlwaysInliner"

[Xun Li via cfe-commits]

Author: Xun Li
Date: 2021-04-18T15:38:19-07:00
New Revision: c0211e8d7d0b797fd11543c3d3f9fecf3b2069cf

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

LOG: Revert "[Coroutines] Move CoroEarly pass to before AlwaysInliner"

This reverts commit 2b50f5a4343f8fb06acaa5c36355bcf58092c9cd.
Forgot to update the description of the commit to sync with phabricator. Going 
to redo the commit.

Added: 


Modified: 
clang/lib/CodeGen/CGCoroutine.cpp
clang/test/CodeGenCoroutines/coro-always-inline.cpp
llvm/lib/Transforms/Coroutines/CoroEarly.cpp
llvm/test/Transforms/Coroutines/coro-debug-O2.ll
llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
llvm/test/Transforms/Coroutines/coro-split-01.ll
llvm/test/Transforms/Coroutines/coro-split-recursive.ll
llvm/test/Transforms/Coroutines/ex0.ll
llvm/test/Transforms/Coroutines/ex1.ll
llvm/test/Transforms/Coroutines/ex2.ll
llvm/test/Transforms/Coroutines/ex3.ll
llvm/test/Transforms/Coroutines/ex4.ll
llvm/test/Transforms/Coroutines/ex5.ll
llvm/test/Transforms/Coroutines/phi-coro-end.ll
llvm/test/Transforms/Coroutines/restart-trigger.ll

Removed: 
clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp



diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index fcf8fe062367f..ca071d3d2e80f 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -558,8 +558,6 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt ) {
   CurCoro.Data->SuspendBB = RetBB;
   assert(ShouldEmitLifetimeMarkers &&
  "Must emit lifetime intrinsics for coroutines");
-  // CORO_PRESPLIT_ATTR = UNPREPARED_FOR_SPLIT
-  CurFn->addFnAttr("coroutine.presplit", "0");
 
   // Backend is allowed to elide memory allocations, to help it, emit
   // auto mem = coro.alloc() ? 0 : ... allocation code ...;

diff  --git a/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp 
b/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
deleted file mode 100644
index e4aa14a6ac397..0
--- a/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
+++ /dev/null
@@ -1,54 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -fexperimental-new-pass-manager -O0 %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -fexperimental-new-pass-manager -fno-inline -O0 %s -o - | FileCheck 
%s
-
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -O0 %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
-// RUN:   -fno-inline -O0 %s -o - | FileCheck %s
-
-namespace std {
-namespace experimental {
-
-struct handle {};
-
-struct awaitable {
-  bool await_ready() noexcept { return true; }
-  // CHECK-NOT: await_suspend
-  inline void __attribute__((__always_inline__)) await_suspend(handle) 
noexcept {}
-  bool await_resume() noexcept { return true; }
-};
-
-template 
-struct coroutine_handle {
-  static handle from_address(void *address) noexcept { return {}; }
-};
-
-template 
-struct coroutine_traits {
-  struct promise_type {
-awaitable initial_suspend() { return {}; }
-awaitable final_suspend() noexcept { return {}; }
-void return_void() {}
-T get_return_object() { return T(); }
-void unhandled_exception() {}
-  };
-};
-} // namespace experimental
-} // namespace std
-
-// CHECK-LABEL: @_Z3foov
-// CHECK-LABEL: entry:
-// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
-// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
-// CHECK: [[CAST0:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST0]])
-// CHECK: [[CAST1:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST1]])
-
-// CHECK: [[CAST2:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST2]])
-// CHECK: [[CAST3:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
-// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST3]])
-void foo() { co_return; }

diff  --git a/clang/test/CodeGenCoroutines/coro-always-inline.cpp 
b/clang/test/CodeGenCoroutines/coro-always-inline.cpp
index 6ba5a6f124169..e4aa14a6ac397 100644
--- a/clang/test/CodeGenCoroutines/coro-always-inline.cpp
+++ 

[PATCH] D100712: [AST] Add NestedNameSpecifierLoc accessors to node introspection

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: clang/unittests/Introspection/IntrospectionTest.cpp:280
 STRING_LOCATION_STDPAIR(MethodDecl, getTypeSpecStartLoc())
   }));
   // clang-format on

njames93 wrote:
> I'm not entirely sure on the case, but the windows bot is giving a [[ 
> https://docs.microsoft.com/en-us/cpp/error-messages/compiler-errors-1/compiler-error-c2026?view=msvc-160
>  | C2026 ]] error here. Not entirely sure why as there is no string over the 
> max length allowed.
After https://reviews.llvm.org/D100720 is in, I'll push an update to fix this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100712

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

[clang] 2b50f5a - [Coroutines] Move CoroEarly pass to before AlwaysInliner

[Xun Li via cfe-commits]

Author: Xun Li
Date: 2021-04-18T14:54:04-07:00
New Revision: 2b50f5a4343f8fb06acaa5c36355bcf58092c9cd

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

LOG: [Coroutines] Move CoroEarly pass to before AlwaysInliner

Presplit coroutines cannot be inlined. During AlwaysInliner we check if a 
function is a presplit coroutine, if so we skip inlining.
The presplit coroutine attributes are set in CoroEarly pass.
However in O0 pipeline, AlwaysInliner runs before CoroEarly, so the attribute 
isn't set yet and will still inline the coroutine.
This causes Clang to crash: https://bugs.llvm.org/show_bug.cgi?id=49920

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

Added: 
clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp

Modified: 
clang/lib/CodeGen/CGCoroutine.cpp
clang/test/CodeGenCoroutines/coro-always-inline.cpp
llvm/lib/Transforms/Coroutines/CoroEarly.cpp
llvm/test/Transforms/Coroutines/coro-debug-O2.ll
llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
llvm/test/Transforms/Coroutines/coro-split-01.ll
llvm/test/Transforms/Coroutines/coro-split-recursive.ll
llvm/test/Transforms/Coroutines/ex0.ll
llvm/test/Transforms/Coroutines/ex1.ll
llvm/test/Transforms/Coroutines/ex2.ll
llvm/test/Transforms/Coroutines/ex3.ll
llvm/test/Transforms/Coroutines/ex4.ll
llvm/test/Transforms/Coroutines/ex5.ll
llvm/test/Transforms/Coroutines/phi-coro-end.ll
llvm/test/Transforms/Coroutines/restart-trigger.ll

Removed: 




diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index ca071d3d2e80f..fcf8fe062367f 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -558,6 +558,8 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt ) {
   CurCoro.Data->SuspendBB = RetBB;
   assert(ShouldEmitLifetimeMarkers &&
  "Must emit lifetime intrinsics for coroutines");
+  // CORO_PRESPLIT_ATTR = UNPREPARED_FOR_SPLIT
+  CurFn->addFnAttr("coroutine.presplit", "0");
 
   // Backend is allowed to elide memory allocations, to help it, emit
   // auto mem = coro.alloc() ? 0 : ... allocation code ...;

diff  --git a/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp 
b/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
new file mode 100644
index 0..e4aa14a6ac397
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fexperimental-new-pass-manager -O0 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fexperimental-new-pass-manager -fno-inline -O0 %s -o - | FileCheck 
%s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -O0 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fcoroutines-ts 
\
+// RUN:   -fno-inline -O0 %s -o - | FileCheck %s
+
+namespace std {
+namespace experimental {
+
+struct handle {};
+
+struct awaitable {
+  bool await_ready() noexcept { return true; }
+  // CHECK-NOT: await_suspend
+  inline void __attribute__((__always_inline__)) await_suspend(handle) 
noexcept {}
+  bool await_resume() noexcept { return true; }
+};
+
+template 
+struct coroutine_handle {
+  static handle from_address(void *address) noexcept { return {}; }
+};
+
+template 
+struct coroutine_traits {
+  struct promise_type {
+awaitable initial_suspend() { return {}; }
+awaitable final_suspend() noexcept { return {}; }
+void return_void() {}
+T get_return_object() { return T(); }
+void unhandled_exception() {}
+  };
+};
+} // namespace experimental
+} // namespace std
+
+// CHECK-LABEL: @_Z3foov
+// CHECK-LABEL: entry:
+// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
+// CHECK-NEXT: %this.addr.i{{[0-9]*}} = alloca 
%"struct.std::experimental::awaitable"*, align 8
+// CHECK: [[CAST0:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST0]])
+// CHECK: [[CAST1:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST1]])
+
+// CHECK: [[CAST2:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[CAST2]])
+// CHECK: [[CAST3:%[0-9]+]] = bitcast %"struct.std::experimental::awaitable"** 
%this.addr.i{{[0-9]*}} to i8*
+// CHECK-NEXT: call void @llvm.lifetime.end.p0i8(i64 8, i8* [[CAST3]])
+void 

[PATCH] D100282: [Coroutines] Set presplit attribute in Clang instead of CoroEarly pass

[Xun Li 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 rG2b50f5a4343f: [Coroutines] Move CoroEarly pass to before 
AlwaysInliner (authored by lxfind).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100282

Files:
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/test/CodeGenCoroutines/coro-always-inline-resume.cpp
  clang/test/CodeGenCoroutines/coro-always-inline.cpp
  llvm/lib/Transforms/Coroutines/CoroEarly.cpp
  llvm/test/Transforms/Coroutines/coro-debug-O2.ll
  llvm/test/Transforms/Coroutines/coro-debug-frame-variable.ll
  llvm/test/Transforms/Coroutines/coro-split-01.ll
  llvm/test/Transforms/Coroutines/coro-split-recursive.ll
  llvm/test/Transforms/Coroutines/ex0.ll
  llvm/test/Transforms/Coroutines/ex1.ll
  llvm/test/Transforms/Coroutines/ex2.ll
  llvm/test/Transforms/Coroutines/ex3.ll
  llvm/test/Transforms/Coroutines/ex4.ll
  llvm/test/Transforms/Coroutines/ex5.ll
  llvm/test/Transforms/Coroutines/phi-coro-end.ll
  llvm/test/Transforms/Coroutines/restart-trigger.ll

Index: llvm/test/Transforms/Coroutines/restart-trigger.ll
===
--- llvm/test/Transforms/Coroutines/restart-trigger.ll
+++ llvm/test/Transforms/Coroutines/restart-trigger.ll
@@ -12,7 +12,7 @@
 ; CHECK:  CoroSplit: Processing coroutine 'f' state: 0
 ; CHECK-NEXT: CoroSplit: Processing coroutine 'f' state: 1
 
-define void @f() {
+define void @f() "coroutine.presplit"="0" {
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
   %size = call i32 @llvm.coro.size.i32()
   %alloc = call i8* @malloc(i32 %size)
Index: llvm/test/Transforms/Coroutines/phi-coro-end.ll
===
--- llvm/test/Transforms/Coroutines/phi-coro-end.ll
+++ llvm/test/Transforms/Coroutines/phi-coro-end.ll
@@ -2,7 +2,7 @@
 ; RUN: opt < %s -O2 -enable-coroutines -S | FileCheck %s
 ; RUN: opt < %s -aa-pipeline=basic-aa -passes='default' -enable-coroutines -S | FileCheck %s
 
-define i8* @f(i32 %n) {
+define i8* @f(i32 %n) "coroutine.presplit"="0" {
 entry:
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
   %size = call i32 @llvm.coro.size.i32()
Index: llvm/test/Transforms/Coroutines/ex5.ll
===
--- llvm/test/Transforms/Coroutines/ex5.ll
+++ llvm/test/Transforms/Coroutines/ex5.ll
@@ -2,7 +2,7 @@
 ; RUN: opt < %s -O2 -enable-coroutines -preserve-alignment-assumptions-during-inlining=false -S | FileCheck %s
 ; RUN: opt < %s -aa-pipeline=basic-aa -passes='default' -enable-coroutines -preserve-alignment-assumptions-during-inlining=false -S | FileCheck %s
 
-define i8* @f(i32 %n) {
+define i8* @f(i32 %n) "coroutine.presplit"="0" {
 entry:
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
   %size = call i32 @llvm.coro.size.i32()
Index: llvm/test/Transforms/Coroutines/ex4.ll
===
--- llvm/test/Transforms/Coroutines/ex4.ll
+++ llvm/test/Transforms/Coroutines/ex4.ll
@@ -2,7 +2,7 @@
 ; RUN: opt < %s -O2 -enable-coroutines -S | FileCheck %s
 ; RUN: opt < %s -passes='default' -enable-coroutines -S | FileCheck %s
 
-define i8* @f(i32 %n) {
+define i8* @f(i32 %n) "coroutine.presplit"="0" {
 entry:
   %promise = alloca i32
   %pv = bitcast i32* %promise to i8*
Index: llvm/test/Transforms/Coroutines/ex3.ll
===
--- llvm/test/Transforms/Coroutines/ex3.ll
+++ llvm/test/Transforms/Coroutines/ex3.ll
@@ -2,7 +2,7 @@
 ; RUN: opt < %s -O2 -enable-coroutines -S | FileCheck %s
 ; RUN: opt < %s -aa-pipeline=basic-aa -passes='default' -enable-coroutines -S | FileCheck %s
 
-define i8* @f(i32 %n) {
+define i8* @f(i32 %n) "coroutine.presplit"="0" {
 entry:
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
   %size = call i32 @llvm.coro.size.i32()
Index: llvm/test/Transforms/Coroutines/ex2.ll
===
--- llvm/test/Transforms/Coroutines/ex2.ll
+++ llvm/test/Transforms/Coroutines/ex2.ll
@@ -2,7 +2,7 @@
 ; RUN: opt < %s -O2 -enable-coroutines -S | FileCheck %s
 ; RUN: opt < %s -passes='default' -enable-coroutines -S | FileCheck %s
 
-define i8* @f(i32 %n) {
+define i8* @f(i32 %n) "coroutine.presplit"="0" {
 entry:
   %id = call token @llvm.coro.id(i32 0, i8* null, i8* null, i8* null)
   %need.dyn.alloc = call i1 @llvm.coro.alloc(token %id)
Index: llvm/test/Transforms/Coroutines/ex1.ll
===
--- llvm/test/Transforms/Coroutines/ex1.ll
+++ llvm/test/Transforms/Coroutines/ex1.ll
@@ -2,7 +2,7 @@
 ; RUN: opt < %s -O2 -enable-coroutines -preserve-alignment-assumptions-during-inlining=false -S | FileCheck %s
 ; RUN: opt < %s 

[PATCH] D100727: [clang-format] Correctly apply AllowShortIfStatementsOnASingleLine: Always to else branch.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

In D100727#2697419 , 
@HazardyKnusperkeks wrote:

> How is
>
>   if (a) return;
>   else
> return;
>
> formatted with the different options?

Do you have something specific in mind?

> And from the documentation I think it was intended that only `if` is short, 
> never the `else`.

There's already an option WithoutElse that should do exactly this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100727

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

[PATCH] D100733: [clang] NFC: change uses of `Expr->getValueKind` into `is?Value`

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov created this revision.
Herald added a subscriber: lxfind.
mizvekov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Signed-off-by: Matheus Izvekov 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100733

Files:
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp

Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5830,7 +5830,7 @@
  Entity.getType()) &&
 canPerformArrayCopy(Entity)) {
   // If source is a prvalue, use it directly.
-  if (Initializer->getValueKind() == VK_RValue) {
+  if (Initializer->isRValue()) {
 AddArrayInitStep(DestType, /*IsGNUExtension*/false);
 return;
   }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5513,7 +5513,7 @@
 BaseExpr = LHSExp;// vectors: V[123]
 IndexExpr = RHSExp;
 // We apply C++ DR1213 to vector subscripting too.
-if (getLangOpts().CPlusPlus11 && LHSExp->getValueKind() == VK_RValue) {
+if (getLangOpts().CPlusPlus11 && LHSExp->isRValue()) {
   ExprResult Materialized = TemporaryMaterializationConversion(LHSExp);
   if (Materialized.isInvalid())
 return ExprError();
@@ -10093,7 +10093,7 @@
 RHSType, DiagID))
 return RHSType;
 } else {
-  if (LHS.get()->getValueKind() == VK_LValue ||
+  if (LHS.get()->isLValue() ||
   !tryGCCVectorConvertAndSplat(*this, , ))
 return RHSType;
 }
@@ -14816,7 +14816,7 @@
 // complex l-values to ordinary l-values and all other values to r-values.
 if (Input.isInvalid()) return ExprError();
 if (Opc == UO_Real || Input.get()->getType()->isAnyComplexType()) {
-  if (Input.get()->getValueKind() != VK_RValue &&
+  if (!Input.get()->isRValue() &&
   Input.get()->getObjectKind() == OK_Ordinary)
 VK = Input.get()->getValueKind();
 } else if (!getLangOpts().CPlusPlus) {
@@ -18985,7 +18985,7 @@
   Expr *SubExpr = SubResult.get();
   E->setSubExpr(SubExpr);
   E->setType(S.Context.getPointerType(SubExpr->getType()));
-  assert(E->getValueKind() == VK_RValue);
+  assert(E->isRValue());
   assert(E->getObjectKind() == OK_Ordinary);
   return E;
 }
@@ -18995,7 +18995,7 @@
 
   E->setType(VD->getType());
 
-  assert(E->getValueKind() == VK_RValue);
+  assert(E->isRValue());
   if (S.getLangOpts().CPlusPlus &&
   !(isa(VD) &&
 cast(VD)->isInstance()))
@@ -19086,7 +19086,7 @@
 return ExprError();
   }
 
-  assert(E->getValueKind() == VK_RValue);
+  assert(E->isRValue());
   assert(E->getObjectKind() == OK_Ordinary);
   E->setType(DestType);
 
@@ -19240,7 +19240,7 @@
 ExprResult RebuildUnknownAnyExpr::VisitImplicitCastExpr(ImplicitCastExpr *E) {
   // The only case we should ever see here is a function-to-pointer decay.
   if (E->getCastKind() == CK_FunctionToPointerDecay) {
-assert(E->getValueKind() == VK_RValue);
+assert(E->isRValue());
 assert(E->getObjectKind() == OK_Ordinary);
 
 E->setType(DestType);
@@ -19254,7 +19254,7 @@
 E->setSubExpr(Result.get());
 return E;
   } else if (E->getCastKind() == CK_LValueToRValue) {
-assert(E->getValueKind() == VK_RValue);
+assert(E->isRValue());
 assert(E->getObjectKind() == OK_Ordinary);
 
 assert(isa(E->getType()));
Index: clang/lib/Sema/SemaCoroutine.cpp
===
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -897,7 +897,7 @@
 
   // If the expression is a temporary, materialize it as an lvalue so that we
   // can use it multiple times.
-  if (E->getValueKind() == VK_RValue)
+  if (E->isRValue())
 E = CreateMaterializeTemporaryExpr(E->getType(), E, true);
 
   // The location of the `co_await` token cannot be used when constructing
@@ -957,7 +957,7 @@
 
   // If the expression is a temporary, materialize it as an lvalue so that we
   // can use it multiple times.
-  if (E->getValueKind() == VK_RValue)
+  if (E->isRValue())
 E = CreateMaterializeTemporaryExpr(E->getType(), E, true);
 
   // Build the await_ready, await_suspend, await_resume calls.
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -614,7 +614,7 @@
   // C++1z [conv.array]: The temporary materialization conversion is applied.
   // We also use this to fuel 

[PATCH] D100713: [clang] NFC: refactor usage of getDecltypeForParenthesizedExpr and getValueKind

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 338393.
mizvekov added a comment.

Split 'getValueKind' cleanup into a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100713

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ExprObjC.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp

Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -69,29 +69,7 @@
 
 QualType CallEvent::getResultType() const {
   ASTContext  = getState()->getStateManager().getContext();
-  const Expr *E = getOriginExpr();
-  if (!E)
-return Ctx.VoidTy;
-  assert(E);
-
-  QualType ResultTy = E->getType();
-
-  // A function that returns a reference to 'int' will have a result type
-  // of simply 'int'. Check the origin expr's value kind to recover the
-  // proper type.
-  switch (E->getValueKind()) {
-  case VK_LValue:
-ResultTy = Ctx.getLValueReferenceType(ResultTy);
-break;
-  case VK_XValue:
-ResultTy = Ctx.getRValueReferenceType(ResultTy);
-break;
-  case VK_RValue:
-// No adjustment is necessary.
-break;
-  }
-
-  return ResultTy;
+  return Ctx.getDecltypeForParenthesizedExpr(getOriginExpr());
 }
 
 static bool isCallback(QualType T) {
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -8830,29 +8830,6 @@
   return Context.getTypeOfExprType(E);
 }
 
-/// getDecltypeForParenthesizedExpr - Given an expr, will return the type for
-/// that expression, as in [dcl.type.simple]p4 but without taking id-expressions
-/// and class member access into account.
-QualType Sema::getDecltypeForParenthesizedExpr(Expr *E) {
-  // C++11 [dcl.type.simple]p4:
-  //   [...]
-  QualType T = E->getType();
-  switch (E->getValueKind()) {
-  // - otherwise, if e is an xvalue, decltype(e) is T&&, where T is the
-  //   type of e;
-  case VK_XValue:
-return Context.getRValueReferenceType(T);
-  // - otherwise, if e is an lvalue, decltype(e) is T&, where T is the
-  //   type of e;
-  case VK_LValue:
-return Context.getLValueReferenceType(T);
-  //  - otherwise, decltype(e) is the type of e.
-  case VK_RValue:
-return T;
-  }
-  llvm_unreachable("Unknown value kind");
-}
-
 /// getDecltypeForExpr - Given an expr, will return the decltype for
 /// that expression, according to the rules in C++11
 /// [dcl.type.simple]p4 and C++11 [expr.lambda.prim]p18.
@@ -8917,7 +8894,7 @@
 }
   }
 
-  return S.getDecltypeForParenthesizedExpr(E);
+  return S.Context.getDecltypeForParenthesizedExpr(E);
 }
 
 QualType Sema::BuildDecltypeType(Expr *E, SourceLocation Loc,
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5841,10 +5841,8 @@
   //   -- If E2 is an xvalue: E1 can be converted to match E2 if E1 can be
   //  implicitly converted to the type "rvalue reference to R2", subject to
   //  the constraint that the reference must bind directly.
-  if (To->isLValue() || To->isXValue()) {
-QualType T = To->isLValue() ? Self.Context.getLValueReferenceType(ToType)
-: Self.Context.getRValueReferenceType(ToType);
-
+  if (!To->isRValue()) {
+QualType T = Self.Context.getDecltypeForParenthesizedExpr(To);
 InitializedEntity Entity = InitializedEntity::InitializeTemporary(T);
 
 InitializationSequence InitSeq(Self, Entity, Kind, From);
@@ -8663,7 +8661,7 @@
 TemplateParameterList *TPL =
 ReturnTypeRequirement.getTypeConstraintTemplateParameterList();
 QualType MatchedType =
-getDecltypeForParenthesizedExpr(E).getCanonicalType();
+Context.getDecltypeForParenthesizedExpr(E).getCanonicalType();
 llvm::SmallVector Args;
 Args.push_back(TemplateArgument(MatchedType));
 TemplateArgumentList TAL(TemplateArgumentList::OnStack, Args);
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19180,14 +19180,8 @@
 if (ParamTypes.empty() && Proto->isVariadic()) { // the special case
   ArgTypes.reserve(E->getNumArgs());
   for (unsigned i = 0, e = E->getNumArgs(); i != e; ++i) {
-Expr *Arg = E->getArg(i);
-QualType ArgType = Arg->getType();
-if (E->isLValue()) {
-  ArgType = S.Context.getLValueReferenceType(ArgType);
-} else if (E->isXValue()) {
-  ArgType = 

[PATCH] D99861: [Clang] Record tokens in attribute arguments for user-defined C++/C2x attributes

[Josh Junon via Phabricator via cfe-commits]

Qix- added a comment.

I'm not sure exactly how to continue after the last few comments - what should 
the approach be for this patch? Or are these things we can shoot for in later 
patches?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99861

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

[PATCH] D100719: [Introspection] Dont emit json if unchanged.

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdb75db85f231: [Introspection] Dont emit json if unchanged. 
(authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100719

Files:
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp


Index: clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
===
--- clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
+++ clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
@@ -10,6 +10,7 @@
 
 #include "clang/Frontend/CompilerInstance.h"
 #include "llvm/Support/JSON.h"
+#include "llvm/Support/MemoryBuffer.h"
 
 using namespace clang::tooling;
 using namespace llvm;
@@ -105,13 +106,27 @@
   JsonObj["classesInClade"] = std::move(ClassesInClade);
   JsonObj["classEntries"] = std::move(ClassEntries);
 
+  llvm::json::Value JsonVal(std::move(JsonObj));
+
+  bool WriteChange = false;
+  std::string OutString;
+  if (auto ExistingOrErr = MemoryBuffer::getFile(JsonPath, /*IsText=*/true)) {
+raw_string_ostream Out(OutString);
+Out << formatv("{0:2}", JsonVal);
+if (ExistingOrErr.get()->getBuffer() == Out.str())
+  return;
+WriteChange = true;
+  }
+
   std::error_code EC;
   llvm::raw_fd_ostream JsonOut(JsonPath, EC, llvm::sys::fs::F_Text);
   if (EC)
 return;
 
-  llvm::json::Value JsonVal(std::move(JsonObj));
-  JsonOut << formatv("{0:2}", JsonVal);
+  if (WriteChange)
+JsonOut << OutString;
+  else
+JsonOut << formatv("{0:2}", JsonVal);
 }
 
 void ASTSrcLocProcessor::generate() {


Index: clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
===
--- clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
+++ clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
@@ -10,6 +10,7 @@
 
 #include "clang/Frontend/CompilerInstance.h"
 #include "llvm/Support/JSON.h"
+#include "llvm/Support/MemoryBuffer.h"
 
 using namespace clang::tooling;
 using namespace llvm;
@@ -105,13 +106,27 @@
   JsonObj["classesInClade"] = std::move(ClassesInClade);
   JsonObj["classEntries"] = std::move(ClassEntries);
 
+  llvm::json::Value JsonVal(std::move(JsonObj));
+
+  bool WriteChange = false;
+  std::string OutString;
+  if (auto ExistingOrErr = MemoryBuffer::getFile(JsonPath, /*IsText=*/true)) {
+raw_string_ostream Out(OutString);
+Out << formatv("{0:2}", JsonVal);
+if (ExistingOrErr.get()->getBuffer() == Out.str())
+  return;
+WriteChange = true;
+  }
+
   std::error_code EC;
   llvm::raw_fd_ostream JsonOut(JsonPath, EC, llvm::sys::fs::F_Text);
   if (EC)
 return;
 
-  llvm::json::Value JsonVal(std::move(JsonObj));
-  JsonOut << formatv("{0:2}", JsonVal);
+  if (WriteChange)
+JsonOut << OutString;
+  else
+JsonOut << formatv("{0:2}", JsonVal);
 }
 
 void ASTSrcLocProcessor::generate() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] db75db8 - [Introspection] Dont emit json if unchanged.

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2021-04-18T20:22:09+01:00
New Revision: db75db85f231bf194912c3b0dd918f2bc497d603

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

LOG: [Introspection] Dont emit json if unchanged.

Saves running the generate inc script in the, somewhat common, case where the 
json file doesn't need changing.

Reviewed By: steveire

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

Added: 


Modified: 
clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp

Removed: 




diff  --git a/clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp 
b/clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
index 8ad187eedfb0..a06f4ff9c4a3 100644
--- a/clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
+++ b/clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
@@ -10,6 +10,7 @@
 
 #include "clang/Frontend/CompilerInstance.h"
 #include "llvm/Support/JSON.h"
+#include "llvm/Support/MemoryBuffer.h"
 
 using namespace clang::tooling;
 using namespace llvm;
@@ -105,13 +106,27 @@ void WriteJSON(StringRef JsonPath, llvm::json::Object 
&,
   JsonObj["classesInClade"] = std::move(ClassesInClade);
   JsonObj["classEntries"] = std::move(ClassEntries);
 
+  llvm::json::Value JsonVal(std::move(JsonObj));
+
+  bool WriteChange = false;
+  std::string OutString;
+  if (auto ExistingOrErr = MemoryBuffer::getFile(JsonPath, /*IsText=*/true)) {
+raw_string_ostream Out(OutString);
+Out << formatv("{0:2}", JsonVal);
+if (ExistingOrErr.get()->getBuffer() == Out.str())
+  return;
+WriteChange = true;
+  }
+
   std::error_code EC;
   llvm::raw_fd_ostream JsonOut(JsonPath, EC, llvm::sys::fs::F_Text);
   if (EC)
 return;
 
-  llvm::json::Value JsonVal(std::move(JsonObj));
-  JsonOut << formatv("{0:2}", JsonVal);
+  if (WriteChange)
+JsonOut << OutString;
+  else
+JsonOut << formatv("{0:2}", JsonVal);
 }
 
 void ASTSrcLocProcessor::generate() {



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

[PATCH] D100712: [AST] Add NestedNameSpecifierLoc accessors to node introspection

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang/unittests/Introspection/IntrospectionTest.cpp:280
 STRING_LOCATION_STDPAIR(MethodDecl, getTypeSpecStartLoc())
   }));
   // clang-format on

I'm not entirely sure on the case, but the windows bot is giving a [[ 
https://docs.microsoft.com/en-us/cpp/error-messages/compiler-errors-1/compiler-error-c2026?view=msvc-160
 | C2026 ]] error here. Not entirely sure why as there is no string over the 
max length allowed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100712

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

[PATCH] D100727: [clang-format] Correctly apply AllowShortIfStatementsOnASingleLine: Always to else branch.

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

HazardyKnusperkeks added a comment.

How is

  if (a) return;
  else
return;

formatted with the different options?

And from the documentation I think it was intended that only `if` is short, 
never the `else`. So I think it behaves correctly, nevertheless I think it 
should be possible, but I don't know wether with new enumerators or a new 
option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100727

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

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

To answer one of your questions above, if there is a function definition 
before, dyn_cast(...) will return nullptr. I tried starting 
from "Value" class and found dyn_cast to llvm::ConstantExpr will result in a 
non-null object, but I did not trace down further with subclasses of 
llvm::ConstantExpr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100567

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

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

In D100567#2697412 , @dblaikie wrote:

> In D100567#2696095 , @yonghong-song 
> wrote:
>
>> Sorry, I know what is the segfault now after some debugging. It is because 
>> `auto *Fn = dyn_cast(LV.getPointer(*this));` is a NULL 
>> pointer after there is a definition before this.
>
> Hmm, not sure I follow - is `LV.getPointer(*this)` returning null, or is the 
> dyn_cast returning null because the thing isn't an llvm::Function? If it's 
> not a function, what is it?

The patched code is `LV.getPointer(*this)->dump();`, so `LV.getPointer(*this)` 
is not return NULL. It returns a valid object. If the function already has a 
definition, it returns a llvm:ConstantExpr,
and if you call the `LV.getPointer(*this)->dump()`, and it prints `i32 (...)* 
bitcast (i32 ()* @foo to i32 (...)*)`. I didn't try subclasses of 
"llvm:ConstantExpr". I think you may have a better idea
what the real object could be?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100567

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

[PATCH] D100567: BPF: emit debuginfo for Function of DeclRefExpr if requested

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D100567#2696095 , @yonghong-song 
wrote:

> Sorry, I know what is the segfault now after some debugging. It is because 
> `auto *Fn = dyn_cast(LV.getPointer(*this));` is a NULL 
> pointer after there is a definition before this.

Hmm, not sure I follow - is `LV.getPointer(*this)` returning null, or is the 
dyn_cast returning null because the thing isn't an llvm::Function? If it's not 
a function, what is it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100567

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

[PATCH] D100415: [Coroutines] Split coroutine during CoroEarly into an init and ramp function

[Xun Li via Phabricator via cfe-commits]

lxfind added inline comments.



Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:2231
 // coroutine.
 struct CoroSplitLegacy : public CallGraphSCCPass {
   static char ID; // Pass identification, replacement for typeid

ChuanqiXu wrote:
> I am not familiar with the policy in LLVM that how should we treat LegacyPass 
> in trunk. I mean, are we responsible to update the LegacyPassManager?
Yes I think so. I will deal with the legacypass latter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100415

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

[PATCH] D100712: [AST] Add NestedNameSpecifierLoc accessors to node introspection

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py:57
 InstanceDecoration = "*"
-if CladeName == "TypeLoc":
+if CladeName == "TypeLoc" or CladeName == "NestedNameSpecifierLoc":
 InstanceDecoration = "&"

njames93 wrote:
> Are there more Clades that require const ref instead of pointer, it may be 
> wise to create a set of these and re use that throughout
> ```lang=py
> RefClades = { "TypeLoc", "NestedNameSpecifierLoc" }
> ...
> InstanceDecoration = '&' if CladeName in RefClades else '*'
> ```
Split into https://reviews.llvm.org/D100720



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100712

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

[PATCH] D100723: [AST] Fix comparison to of SourceRanges in container

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: clang/lib/Tooling/NodeIntrospection.cpp:47
 std::pair const ) const {
-  if (!LHS.first.isValid() || !RHS.first.isValid())
-return false;

njames93 wrote:
> Maybe we should assert the ranges (or locations) are valid before inserting. 
> Will require modification to the generator script but it would make 
> subsequent handling simpler. 
I don't think I agree, but I think that's a different patch. We should fix the 
bug for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100723

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

[PATCH] D100727: [clang-format] Correctly apply AllowShortIfStatementsOnASingleLine: Always to else branch.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius created this revision.
curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks.
curdeius requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This fixes a bug reported on Discord #clang-format 
. I'll update the summary as soon 
as I have the bugzilla ticket number.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100727

Files:
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -510,6 +510,60 @@
 
   AllowsMergedIf.ColumnLimit = 13;
   verifyFormat("if (a)\n  return;", AllowsMergedIf);
+
+  FormatStyle AllowsMergedIfElse = getLLVMStyle();
+  AllowsMergedIfElse.AllowShortIfStatementsOnASingleLine =
+  FormatStyle::SIS_Always;
+  verifyFormat("if (a)\n"
+   "  // comment\n"
+   "  f();\n"
+   "else\n"
+   "  // comment\n"
+   "  f();",
+   AllowsMergedIfElse);
+  verifyFormat("{\n"
+   "  if (a)\n"
+   "  label:\n"
+   "f();\n"
+   "  else\n"
+   "  label:\n"
+   "f();\n"
+   "}",
+   AllowsMergedIfElse);
+  // Hum?
+  verifyFormat("if (a)\n"
+   "  ;\n"
+   "else\n"
+   "  ;",
+   AllowsMergedIfElse);
+  verifyFormat("if (a) {\n"
+   "} else {\n"
+   "}",
+   AllowsMergedIfElse);
+  verifyFormat("if (a) return;\n"
+   "else if (b) return;\n"
+   "else return;",
+   AllowsMergedIfElse);
+  verifyFormat("if (a) {\n"
+   "} else return;",
+   AllowsMergedIfElse);
+  verifyFormat("if (a) {\n"
+   "} else if (b) return;\n"
+   "else return;",
+   AllowsMergedIfElse);
+  verifyFormat("if (a) return;\n"
+   "else if (b) {\n"
+   "} else return;",
+   AllowsMergedIfElse);
+  verifyFormat("if (a)\n"
+   "  if (b) return;\n"
+   "  else return;",
+   AllowsMergedIfElse);
+  verifyFormat("if constexpr (a)\n"
+   "  if constexpr (b) return;\n"
+   "  else if constexpr (c) return;\n"
+   "  else return;",
+   AllowsMergedIfElse);
 }
 
 TEST_F(FormatTest, FormatIfWithoutCompoundStatementButElseWith) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -421,7 +421,13 @@
   }
   return MergedLines;
 }
-if (TheLine->First->is(tok::kw_if)) {
+bool IsElseLine =
+TheLine->First->is(tok::kw_else) ||
+(TheLine->First->is(tok::r_brace) && TheLine->First->Next &&
+ TheLine->First->Next->is(tok::kw_else));
+if (TheLine->First->is(tok::kw_if) ||
+(IsElseLine && (Style.AllowShortIfStatementsOnASingleLine ==
+FormatStyle::SIS_Always))) {
   return Style.AllowShortIfStatementsOnASingleLine
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
@@ -471,7 +477,8 @@
   return 0;
 Limit = limitConsideringMacros(I + 1, E, Limit);
 AnnotatedLine  = **I;
-if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren))
+if (!Line.First->is(tok::kw_do) && !Line.First->is(tok::kw_else) &&
+!Line.Last->is(tok::kw_else) && Line.Last->isNot(tok::r_paren))
   return 0;
 // Only merge do while if do is the only statement on the line.
 if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -510,6 +510,60 @@
 
   AllowsMergedIf.ColumnLimit = 13;
   verifyFormat("if (a)\n  return;", AllowsMergedIf);
+
+  FormatStyle AllowsMergedIfElse = getLLVMStyle();
+  AllowsMergedIfElse.AllowShortIfStatementsOnASingleLine =
+  FormatStyle::SIS_Always;
+  verifyFormat("if (a)\n"
+   "  // comment\n"
+   "  f();\n"
+   "else\n"
+   "  // comment\n"
+   "  f();",
+   AllowsMergedIfElse);
+  verifyFormat("{\n"
+   "  if (a)\n"
+   "  label:\n"
+   "f();\n"
+   "  else\n"
+   "  label:\n"
+   "f();\n"
+   "}",
+   AllowsMergedIfElse);
+  // Hum?
+  verifyFormat("if (a)\n"
+   "  ;\n"
+   "else\n"
+   " 

[PATCH] D100723: [AST] Fix comparison to of SourceRanges in container

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang/lib/Tooling/NodeIntrospection.cpp:47
 std::pair const ) const {
-  if (!LHS.first.isValid() || !RHS.first.isValid())
-return false;

Maybe we should assert the ranges (or locations) are valid before inserting. 
Will require modification to the generator script but it would make subsequent 
handling simpler. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100723

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

[PATCH] D77013: [AMDGPU] Add options -mamdgpu-ieee -mno-amdgpu-ieee

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 338379.
yaxunl marked 3 inline comments as done.
yaxunl edited the summary of this revision.
yaxunl added a comment.

Revised by Matt's comments.


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

https://reviews.llvm.org/D77013

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenOpenCL/amdgpu-ieee.cl

Index: clang/test/CodeGenOpenCL/amdgpu-ieee.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/amdgpu-ieee.cl
@@ -0,0 +1,48 @@
+// REQUIRES: amdgpu-registered-target
+//
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O0 -emit-llvm -o - %s \
+// RUN:   | FileCheck -check-prefixes=COMMON,ON %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O0 -emit-llvm -o - %s \
+// RUN:   -mamdgpu-ieee | FileCheck -check-prefixes=COMMON,ON %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O0 -emit-llvm -o - %s \
+// RUN:   -mno-amdgpu-ieee | FileCheck -check-prefixes=COMMON,OFF %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O0 -emit-llvm -o - %s \
+// RUN:   -menable-no-nans | FileCheck -check-prefixes=COMMON,OFF %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O0 -emit-llvm -o - %s \
+// RUN:   -menable-no-nans -mamdgpu-ieee | FileCheck -check-prefixes=COMMON,ON %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O0 -emit-llvm -o - %s \
+// RUN:   -cl-fast-relaxed-math | FileCheck -check-prefixes=COMMON,OFF %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O0 -emit-llvm -o - %s \
+// RUN:   -cl-fast-relaxed-math -mamdgpu-ieee \
+
+// Check AMDGCN ISA generation.
+
+// RUN: | FileCheck -check-prefixes=COMMON,ON %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O3 -S -o - %s \
+// RUN:   -mamdgpu-ieee \
+// RUN: | FileCheck -check-prefixes=ISA-ON %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -O3 -S -o - %s \
+// RUN:   -mno-amdgpu-ieee \
+// RUN: | FileCheck -check-prefixes=ISA-OFF %s
+
+// COMMON: define{{.*}} amdgpu_kernel void @kern{{.*}} [[ATTRS1:#[0-9]+]]
+// ISA-ON: v_mul_f32_e64 v{{[0-9]+}}, 1.0, s{{[0-9]+}}
+// ISA-ON: v_mul_f32_e64 v{{[0-9]+}}, 1.0, s{{[0-9]+}}
+// ISA-ON: v_min_f32_e32
+// ISA-ON: ; IeeeMode: 1
+// ISA-OFF-NOT: v_mul_f32_e64 v{{[0-9]+}}, 1.0, s{{[0-9]+}}
+// ISA-OFF-NOT: v_mul_f32_e64 v{{[0-9]+}}, 1.0, s{{[0-9]+}}
+// ISA-OFF: v_min_f32_e32
+// ISA-OFF: ; IeeeMode: 0
+kernel void kern(global float *x, float y, float z) {
+  *x = __builtin_fmin(y, z);
+}
+
+// COMMON: define{{.*}}void @fun() [[ATTRS2:#[0-9]+]]
+void fun() {
+}
+
+// ON-NOT: attributes [[ATTRS1]] = {{.*}} "amdgpu-ieee"
+// OFF: attributes [[ATTRS1]] = {{.*}} "amdgpu-ieee"="false"
+// ON-NOT: attributes [[ATTRS2]] = {{.*}} "amdgpu-ieee"
+// OFF: attributes [[ATTRS2]] = {{.*}} "amdgpu-ieee"="false"
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1944,6 +1944,13 @@
   else if (Args.hasArg(options::OPT_fno_finite_loops))
 Opts.FiniteLoops = CodeGenOptions::FiniteLoopsKind::Never;
 
+  // When NaN is not honored, floating point opcodes that support exception
+  // flag gathering does not need to quiet or propagate signaling NaN inputs
+  // per IEEE 754-2008. Note this only concerns about signaling NaN.
+  Opts.EmitIEEENaNCompliantInsts =
+  Args.hasFlag(options::OPT_mamdgpu_ieee, options::OPT_mno_amdgpu_ieee,
+   !LangOptsRef.NoHonorNaNs);
+
   return Success && Diags.getNumErrors() == NumErrorsBefore;
 }
 
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -9166,6 +9166,9 @@
 
   if (M.getContext().getTargetInfo().allowAMDGPUUnsafeFPAtomics())
 F->addFnAttr("amdgpu-unsafe-fp-atomics", "true");
+
+  if (!getABIInfo().getCodeGenOpts().EmitIEEENaNCompliantInsts)
+F->addFnAttr("amdgpu-ieee", "false");
 }
 
 unsigned AMDGPUTargetCodeGenInfo::getOpenCLKernelCallingConv() const {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3165,6 +3165,12 @@
  Values<"command,reactor">,
  HelpText<"Execution model (WebAssembly only)">;
 
+def mamdgpu_ieee : Flag<["-"], "mamdgpu-ieee">, Flags<[CC1Option]>,
+  Group, HelpText<"Floating point opcodes that support exception flag "
+   "gathering quiet and propagate signaling NaN inputs per IEEE 754-2008 (AMDGPU only)">;
+def mno_amdgpu_ieee : Flag<["-"], "mno-amdgpu-ieee">, Flags<[CC1Option]>,
+  Group;
+
 def mcode_object_version_EQ : Joined<["-"], "mcode-object-version=">, Group,
   HelpText<"Specify code object ABI version. Defaults 

[PATCH] D77013: [AMDGPU] Add options -mamdgpu-ieee -mno-amdgpu-ieee

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 4 inline comments as done.
yaxunl added inline comments.
Herald added subscribers: jansvoboda11, dexonsmith, dang.



Comment at: clang/include/clang/Basic/CodeGenOptions.def:399
 
+/// Whether to emit IEEE754-2008 NaN compliant instructions if available 
(AMDGPU Only)
+CODEGENOPT(EmitIEEENaNCompliantInsts, 1, 1)

arsenm wrote:
> Description is misleading. Better description would be the first line from 
> the manual, 
> "Floating point opcodes that support exception flag gathering quiet and 
> propagate sig- naling NaN inputs per IEEE 754-2008"
will do



Comment at: clang/include/clang/Driver/Options.td:2406
+def mamdgpu_ieee : Flag<["-"], "mamdgpu-ieee">, Flags<[CC1Option]>,
+  Group, HelpText<"Enable IEEE754-2008 NaN compliance in supported 
AMDGPU instructions">;
+def mno_amdgpu_ieee : Flag<["-"], "mno-amdgpu-ieee">, Flags<[CC1Option]>,

arsenm wrote:
> Ditto
will do



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:1434
+
+  Opts.EmitIEEENaNCompliantInsts =
+  Args.hasFlag(options::OPT_mamdgpu_ieee, options::OPT_mno_amdgpu_ieee,

arsenm wrote:
> Add a comment explaining why to turn it off? Also should note this is only 
> really concerns signaling nans
will do



Comment at: clang/test/CodeGenOpenCL/amdgpu-ieee.cl:20
+}
+
+// ON-NOT: attributes [[ATTRS]] = {{.*}} "amdgpu-ieee"

arsenm wrote:
> arsenm wrote:
> > Should also test a non-kernel function
> I think we should also have some ISA check run lines that show the final 
> result for minnum/maxnum lowering, as well as if output modifiers are used
will do


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

https://reviews.llvm.org/D77013

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

[PATCH] D100638: [AST][Introspection] Avoid creating temporary strings when comparing LocationCalls.

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

Your implementation is getting very complicated and it requires many comments.

Also, if we get to this point in the execution of `RangeLessThan::operator()`, 
you're creating and populating two vectors for every two elements contained. I 
know it's `llvm::SmallVector` and maybe not so expensive etc, but doing 
something is more expensive than not doing it.

You're trying to sort elements (which have the same location/range) 
alphabetically according to how it will be sorted after c++ formatting. Sorting 
according to c++ formatting means sorting incorrectly according to JS/python. 
This is simple to demonstrate:

  TEST(Introspection, SourceLocations_CallContainer3) {
  
SourceLocationMap slm;
SharedLocationCall Prefix;
auto call1 = llvm::makeIntrusiveRefCnt(
  
llvm::makeIntrusiveRefCnt(
  
llvm::makeIntrusiveRefCnt(
  Prefix, ""), "", 
LocationCall::IsCast), "");
auto call2 = llvm::makeIntrusiveRefCnt(
  
llvm::makeIntrusiveRefCnt(
  
llvm::makeIntrusiveRefCnt(
  Prefix, ""), "", 
LocationCall::IsCast), "");
slm.insert(
std::make_pair(SourceLocation(), call1));
slm.insert(
std::make_pair(SourceLocation(), call2));
  
for (auto& item : slm)
{
  llvm::errs() << LocationCallFormatterCpp::format(*item.second) << "\n";
}
for (auto& item : slm)
{
  llvm::errs() << LocationCallFormatterSimple::format(*item.second) << "\n";
}
  }

output:

  ().().()
  ().().()
  ().()
  ().()

So, the cost of creating two vectors for each invocation of `lessThan` that 
reaches this point delivers no benefit.

Here is an implementation which does not create those vectors:

  diff --git a/clang/lib/Tooling/NodeIntrospection.cpp 
b/clang/lib/Tooling/NodeIntrospection.cpp
  index 8e2d446c3efe..6e846acf9ecf 100644
  --- a/clang/lib/Tooling/NodeIntrospection.cpp
  +++ b/clang/lib/Tooling/NodeIntrospection.cpp
  @@ -40,6 +40,22 @@ std::string LocationCallFormatterCpp::format(const 
LocationCall ) {
 return Result;
   }
   
  +bool locationLessThan(const LocationCall *LHS, const LocationCall *RHS)
  +{
  +  if (!LHS && !RHS)
  +return false;
  +  if (LHS && !RHS)
  +return true;
  +  if (!LHS && RHS)
  +return false;
  +  auto compareResult = LHS->name().compare(RHS->name());
  +  if (compareResult < 0)
  +return true;
  +  if (compareResult > 0)
  +return false;
  +  return locationLessThan(LHS->on(), RHS->on());
  +}
  +
   namespace internal {
   bool RangeLessThan::operator()(
   std::pair const ,
  @@ -54,15 +70,13 @@ bool RangeLessThan::operator()(
 else if (LHS.first.getEnd() != RHS.first.getEnd())
   return false;
   
  -  return LocationCallFormatterCpp::format(*LHS.second) <
  - LocationCallFormatterCpp::format(*RHS.second);
  +  return locationLessThan(LHS.second.get(), RHS.second.get());
   }
   bool RangeLessThan::operator()(
   std::pair const ,
   std::pair const ) const {
 if (LHS.first == RHS.first)
  -return LocationCallFormatterCpp::format(*LHS.second) <
  -   LocationCallFormatterCpp::format(*RHS.second);
  +return locationLessThan(LHS.second.get(), RHS.second.get());
 return LHS.first < RHS.first;
   }
   } // namespace internal


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100638

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

[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

@teemperor, could you take another look at this patch -- I believe most of your 
concerns are addressed now.


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

https://reviews.llvm.org/D96033

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

[clang] d4528cb - [clang] Fix cross compiling clang for windows after 141945f950e2f3f

[Martin Storsjö via cfe-commits]

Author: Martin Storsjö
Date: 2021-04-18T15:56:18+03:00
New Revision: d4528cbb0e703ee17ee6fb4abe72b7246ccb38f1

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

LOG: [clang] Fix cross compiling clang for windows after 141945f950e2f3f

Don't try to execute clang-ast-dump when cross compiling.

Added: 


Modified: 
clang/lib/Tooling/CMakeLists.txt

Removed: 




diff  --git a/clang/lib/Tooling/CMakeLists.txt 
b/clang/lib/Tooling/CMakeLists.txt
index dfb732371dfb..a0bb108a2b6c 100644
--- a/clang/lib/Tooling/CMakeLists.txt
+++ b/clang/lib/Tooling/CMakeLists.txt
@@ -25,6 +25,7 @@ string(CONCAT BINARY_INCLUDE_DIR ${PATH_HEAD} 
"/include/clang/" ${PATH_TAIL})
 
 if (NOT Python3_EXECUTABLE
 OR APPLE
+OR CMAKE_CROSSCOMPILING
 OR GENERATOR_IS_MULTI_CONFIG
 OR NOT LLVM_NATIVE_ARCH IN_LIST LLVM_TARGETS_TO_BUILD
 OR NOT X86 IN_LIST LLVM_TARGETS_TO_BUILD



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

[PATCH] D100723: [AST] Fix comparison to of SourceRanges in container

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added a reviewer: njames93.
steveire requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100723

Files:
  clang/lib/Tooling/NodeIntrospection.cpp
  clang/unittests/Introspection/IntrospectionTest.cpp


Index: clang/unittests/Introspection/IntrospectionTest.cpp
===
--- clang/unittests/Introspection/IntrospectionTest.cpp
+++ clang/unittests/Introspection/IntrospectionTest.cpp
@@ -91,6 +91,20 @@
   EXPECT_EQ(slm.size(), 2u);
 }
 
+TEST(Introspection, SourceLocations_CallContainer2) {
+  SourceRangeMap slm;
+  SharedLocationCall Prefix;
+  slm.insert(
+  std::make_pair(SourceRange(), llvm::makeIntrusiveRefCnt(
+Prefix, "getCXXOperatorNameRange")));
+  EXPECT_EQ(slm.size(), 1u);
+
+  slm.insert(std::make_pair(
+  SourceRange(),
+  llvm::makeIntrusiveRefCnt(Prefix, "getSourceRange")));
+  EXPECT_EQ(slm.size(), 2u);
+}
+
 TEST(Introspection, SourceLocations_CallChainFormatting) {
   SharedLocationCall Prefix;
   auto chainedCall = llvm::makeIntrusiveRefCnt(
Index: clang/lib/Tooling/NodeIntrospection.cpp
===
--- clang/lib/Tooling/NodeIntrospection.cpp
+++ clang/lib/Tooling/NodeIntrospection.cpp
@@ -44,9 +44,6 @@
 bool RangeLessThan::operator()(
 std::pair const ,
 std::pair const ) const {
-  if (!LHS.first.isValid() || !RHS.first.isValid())
-return false;
-
   if (LHS.first.getBegin() < RHS.first.getBegin())
 return true;
   else if (LHS.first.getBegin() != RHS.first.getBegin())


Index: clang/unittests/Introspection/IntrospectionTest.cpp
===
--- clang/unittests/Introspection/IntrospectionTest.cpp
+++ clang/unittests/Introspection/IntrospectionTest.cpp
@@ -91,6 +91,20 @@
   EXPECT_EQ(slm.size(), 2u);
 }
 
+TEST(Introspection, SourceLocations_CallContainer2) {
+  SourceRangeMap slm;
+  SharedLocationCall Prefix;
+  slm.insert(
+  std::make_pair(SourceRange(), llvm::makeIntrusiveRefCnt(
+Prefix, "getCXXOperatorNameRange")));
+  EXPECT_EQ(slm.size(), 1u);
+
+  slm.insert(std::make_pair(
+  SourceRange(),
+  llvm::makeIntrusiveRefCnt(Prefix, "getSourceRange")));
+  EXPECT_EQ(slm.size(), 2u);
+}
+
 TEST(Introspection, SourceLocations_CallChainFormatting) {
   SharedLocationCall Prefix;
   auto chainedCall = llvm::makeIntrusiveRefCnt(
Index: clang/lib/Tooling/NodeIntrospection.cpp
===
--- clang/lib/Tooling/NodeIntrospection.cpp
+++ clang/lib/Tooling/NodeIntrospection.cpp
@@ -44,9 +44,6 @@
 bool RangeLessThan::operator()(
 std::pair const ,
 std::pair const ) const {
-  if (!LHS.first.isValid() || !RHS.first.isValid())
-return false;
-
   if (LHS.first.getBegin() < RHS.first.getBegin())
 return true;
   else if (LHS.first.getBegin() != RHS.first.getBegin())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100719: [Introspection] Dont emit json if unchanged.

[Stephen Kelly via Phabricator via cfe-commits]

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

Great, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100719

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

[PATCH] D100720: [AST] Update introspection API to use const-ref for copyable types

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added a reviewer: njames93.
Herald added a subscriber: mgorny.
steveire requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100720

Files:
  clang/include/clang/Tooling/NodeIntrospection.h
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
  clang/unittests/Introspection/IntrospectionTest.cpp

Index: clang/unittests/Introspection/IntrospectionTest.cpp
===
--- clang/unittests/Introspection/IntrospectionTest.cpp
+++ clang/unittests/Introspection/IntrospectionTest.cpp
@@ -298,7 +298,7 @@
 
   const auto *NNS = BoundNodes[0].getNodeAs("nns");
 
-  auto Result = NodeIntrospection::GetLocations(NNS);
+  auto Result = NodeIntrospection::GetLocations(*NNS);
 
   auto ExpectedLocations =
   FormatExpected(Result.LocationAccessors);
@@ -352,7 +352,7 @@
 
   const auto *TA = BoundNodes[0].getNodeAs("ta");
 
-  auto Result = NodeIntrospection::GetLocations(TA);
+  auto Result = NodeIntrospection::GetLocations(*TA);
 
   auto ExpectedLocations =
   FormatExpected(Result.LocationAccessors);
@@ -407,7 +407,7 @@
 
   const auto *TA = BoundNodes[0].getNodeAs("ta");
 
-  auto Result = NodeIntrospection::GetLocations(TA);
+  auto Result = NodeIntrospection::GetLocations(*TA);
 
   auto ExpectedLocations =
   FormatExpected(Result.LocationAccessors);
@@ -444,7 +444,7 @@
 
   const auto *TA = BoundNodes[0].getNodeAs("ta");
 
-  auto Result = NodeIntrospection::GetLocations(TA);
+  auto Result = NodeIntrospection::GetLocations(*TA);
 
   auto ExpectedLocations =
   FormatExpected(Result.LocationAccessors);
@@ -480,7 +480,7 @@
 
   const auto *TA = BoundNodes[0].getNodeAs("ta");
 
-  auto Result = NodeIntrospection::GetLocations(TA);
+  auto Result = NodeIntrospection::GetLocations(*TA);
 
   auto ExpectedLocations =
   FormatExpected(Result.LocationAccessors);
@@ -517,7 +517,7 @@
 
   const auto *TA = BoundNodes[0].getNodeAs("ta");
 
-  auto Result = NodeIntrospection::GetLocations(TA);
+  auto Result = NodeIntrospection::GetLocations(*TA);
 
   auto ExpectedLocations =
   FormatExpected(Result.LocationAccessors);
@@ -555,7 +555,7 @@
 
   const auto *TA = BoundNodes[0].getNodeAs("ta");
 
-  auto Result = NodeIntrospection::GetLocations(TA);
+  auto Result = NodeIntrospection::GetLocations(*TA);
 
   auto ExpectedLocations =
   FormatExpected(Result.LocationAccessors);
@@ -591,7 +591,7 @@
 
   const auto *TA = BoundNodes[0].getNodeAs("ta");
 
-  auto Result = NodeIntrospection::GetLocations(TA);
+  auto Result = NodeIntrospection::GetLocations(*TA);
 
   auto ExpectedLocations =
   FormatExpected(Result.LocationAccessors);
@@ -628,7 +628,7 @@
 
   const auto *TA = BoundNodes[0].getNodeAs("ta");
 
-  auto Result = NodeIntrospection::GetLocations(TA);
+  auto Result = NodeIntrospection::GetLocations(*TA);
 
   auto ExpectedLocations =
   FormatExpected(Result.LocationAccessors);
Index: clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
===
--- clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
+++ clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py
@@ -11,6 +11,8 @@
 
 implementationContent = ''
 
+RefClades = ["NestedNameSpecifierLoc", "TemplateArgumentLoc", "TypeLoc"]
+
 def __init__(self, templateClasses):
 self.templateClasses = templateClasses
 
@@ -54,7 +56,7 @@
 
 def GenerateBaseGetLocationsDeclaration(self, CladeName):
 InstanceDecoration = "*"
-if CladeName == "TypeLoc":
+if CladeName in self.RefClades:
 InstanceDecoration = "&"
 
 self.implementationContent += \
@@ -164,7 +166,7 @@
 
 MethodReturnType = 'NodeLocationAccessors'
 InstanceDecoration = "*"
-if CladeName == "TypeLoc":
+if CladeName in self.RefClades:
 InstanceDecoration = "&"
 
 Signature = \
@@ -196,7 +198,7 @@
 RecursionGuardParam = ', TypeLocRecursionGuard'
 
 ArgPrefix = '*'
-if CladeName == "TypeLoc":
+if CladeName in self.RefClades:
 ArgPrefix = ''
 self.implementationContent += \
 'GetLocations{0}(Prefix, {1}Object, Locs, Rngs {2});'.format(
@@ -290,7 +292,7 @@
 if (const auto *N = Node.get<{0}>())
 """.format(CladeName)
 ArgPrefix = ""
-if CladeName == "TypeLoc":
+if CladeName in self.RefClades:
 ArgPrefix = "*"
 self.implementationContent += \
 """
@@ -351,11 +353,11 @@
   return {};
 }
 NodeLocationAccessors NodeIntrospection::GetLocations(
-clang::NestedNameSpecifierLoc const*) {
+clang::NestedNameSpecifierLoc const&) {
   return {};
 }
 NodeLocationAccessors NodeIntrospection::GetLocations(
-

[PATCH] D100712: [AST] Add NestedNameSpecifierLoc accessors to node introspection

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang/lib/Tooling/DumpTool/generate_cxx_src_locs.py:57
 InstanceDecoration = "*"
-if CladeName == "TypeLoc":
+if CladeName == "TypeLoc" or CladeName == "NestedNameSpecifierLoc":
 InstanceDecoration = "&"

Are there more Clades that require const ref instead of pointer, it may be wise 
to create a set of these and re use that throughout
```lang=py
RefClades = { "TypeLoc", "NestedNameSpecifierLoc" }
...
InstanceDecoration = '&' if CladeName in RefClades else '*'
```



Comment at: clang/unittests/Introspection/IntrospectionTest.cpp:369-370
+  EXPECT_EQ(
   ExpectedLocations,
-  UnorderedElementsAre(
-  STRING_LOCATION_PAIR(NNS, getBeginLoc()),
-  STRING_LOCATION_PAIR(NNS, getEndLoc()),
-  STRING_LOCATION_PAIR(NNS, getLocalBeginLoc()),
-  STRING_LOCATION_PAIR(NNS, getLocalEndLoc()),
-  STRING_LOCATION_PAIR(
+  (std::vector>{
+  STRING_LOCATION_STDPAIR(NNS, getBeginLoc()),




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100712

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

[PATCH] D100719: [Introspection] Dont emit json if unchanged.

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added a reviewer: steveire.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Saves running the generate inc script in the, somewhat common, case where the 
json file doesn't need changing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D100719

Files:
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp


Index: clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
===
--- clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
+++ clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
@@ -10,6 +10,7 @@
 
 #include "clang/Frontend/CompilerInstance.h"
 #include "llvm/Support/JSON.h"
+#include "llvm/Support/MemoryBuffer.h"
 
 using namespace clang::tooling;
 using namespace llvm;
@@ -105,13 +106,27 @@
   JsonObj["classesInClade"] = std::move(ClassesInClade);
   JsonObj["classEntries"] = std::move(ClassEntries);
 
+  llvm::json::Value JsonVal(std::move(JsonObj));
+
+  bool WriteChange = false;
+  std::string OutString;
+  if (auto ExistingOrErr = MemoryBuffer::getFile(JsonPath, /*IsText=*/true)) {
+raw_string_ostream Out(OutString);
+Out << formatv("{0:2}", JsonVal);
+if (ExistingOrErr.get()->getBuffer() == Out.str())
+  return;
+WriteChange = true;
+  }
+
   std::error_code EC;
   llvm::raw_fd_ostream JsonOut(JsonPath, EC, llvm::sys::fs::F_Text);
   if (EC)
 return;
 
-  llvm::json::Value JsonVal(std::move(JsonObj));
-  JsonOut << formatv("{0:2}", JsonVal);
+  if (WriteChange)
+JsonOut << OutString;
+  else
+JsonOut << formatv("{0:2}", JsonVal);
 }
 
 void ASTSrcLocProcessor::generate() {


Index: clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
===
--- clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
+++ clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
@@ -10,6 +10,7 @@
 
 #include "clang/Frontend/CompilerInstance.h"
 #include "llvm/Support/JSON.h"
+#include "llvm/Support/MemoryBuffer.h"
 
 using namespace clang::tooling;
 using namespace llvm;
@@ -105,13 +106,27 @@
   JsonObj["classesInClade"] = std::move(ClassesInClade);
   JsonObj["classEntries"] = std::move(ClassEntries);
 
+  llvm::json::Value JsonVal(std::move(JsonObj));
+
+  bool WriteChange = false;
+  std::string OutString;
+  if (auto ExistingOrErr = MemoryBuffer::getFile(JsonPath, /*IsText=*/true)) {
+raw_string_ostream Out(OutString);
+Out << formatv("{0:2}", JsonVal);
+if (ExistingOrErr.get()->getBuffer() == Out.str())
+  return;
+WriteChange = true;
+  }
+
   std::error_code EC;
   llvm::raw_fd_ostream JsonOut(JsonPath, EC, llvm::sys::fs::F_Text);
   if (EC)
 return;
 
-  llvm::json::Value JsonVal(std::move(JsonObj));
-  JsonOut << formatv("{0:2}", JsonVal);
+  if (WriteChange)
+JsonOut << OutString;
+  else
+JsonOut << formatv("{0:2}", JsonVal);
 }
 
 void ASTSrcLocProcessor::generate() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D100658: [RISCV] Apply __clang_riscv_builtin_alias to overloaded builtins.

[Zakk Chen via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100658

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

[PATCH] D100611: [RISCV] Add new attribute __clang_riscv_builtin_alias for intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

khchen added a comment.

Look good to me.
BTW, we also need to update the  document 
 
later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100611

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

[clang] a0898f0 - [AST][Introspection][NFC] Remove unnecessary temporary strings.

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2021-04-18T09:25:19+01:00
New Revision: a0898f0cecc78be913e885cfba727bd4d9c49aaf

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

LOG: [AST][Introspection][NFC] Remove unnecessary temporary strings.

Added: 


Modified: 
clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp

Removed: 




diff  --git a/clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp 
b/clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
index 497cd3bdce2ca..8ad187eedfb02 100644
--- a/clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
+++ b/clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
@@ -94,7 +94,7 @@ llvm::json::Object toJSON(llvm::StringMap const 
) {
   return JsonObj;
 }
 
-void WriteJSON(std::string JsonPath, llvm::json::Object &,
+void WriteJSON(StringRef JsonPath, llvm::json::Object &,
llvm::json::Object &,
llvm::json::Object &) {
   llvm::json::Object JsonObj;
@@ -213,20 +213,21 @@ void ASTSrcLocProcessor::run(const 
MatchFinder::MatchResult ) {
 
   const auto  = Templ->getTemplateArgs();
 
-  std::string TArgsString = (DerivedFrom->getName() + "<").str();
+  SmallString<256> TArgsString;
+  llvm::raw_svector_ostream OS(TArgsString);
+  OS << DerivedFrom->getName() << '<';
+
+  clang::PrintingPolicy PPol(Result.Context->getLangOpts());
+  PPol.TerseOutput = true;
 
   for (unsigned I = 0; I < TArgs.size(); ++I) {
-if (I > 0) {
-  TArgsString += ", ";
-}
-auto Ty = TArgs.get(I).getAsType();
-clang::PrintingPolicy PPol(Result.Context->getLangOpts());
-PPol.TerseOutput = true;
-TArgsString += Ty.getAsString(PPol);
+if (I > 0)
+  OS << ", ";
+TArgs.get(I).getAsType().print(OS, PPol);
   }
-  TArgsString += ">";
+  OS << '>';
 
-  ClassInheritance[ClassName] = std::move(TArgsString);
+  ClassInheritance[ClassName] = TArgsString.str().str();
 } else {
   ClassInheritance[ClassName] = DerivedFrom->getName().str();
 }



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

[PATCH] D100638: [AST][Introspection] Avoid creating temporary strings when comparing LocationCalls.

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 338356.
njames93 added a comment.

Rebase and remove Args from comparison.
Add quick check for skippig common prefix calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100638

Files:
  clang/lib/Tooling/NodeIntrospection.cpp


Index: clang/lib/Tooling/NodeIntrospection.cpp
===
--- clang/lib/Tooling/NodeIntrospection.cpp
+++ clang/lib/Tooling/NodeIntrospection.cpp
@@ -13,6 +13,9 @@
 #include "clang/Tooling/NodeIntrospection.h"
 
 #include "clang/AST/AST.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
@@ -40,6 +43,42 @@
   return Result;
 }
 
+/// A 3 way comparison of LHS and RHS based on call names.
+static int compareLocationCall(const LocationCall ,
+   const LocationCall ) {
+  // Expand the call stacks into a vector so we can traverse it in reverse
+  // order.
+  llvm::SmallVector Left, Right;
+  for (const LocationCall *Item =  Item; Item = Item->on())
+Left.push_back(Item);
+  for (const LocationCall *Item =  Item; Item = Item->on())
+Right.push_back(Item);
+
+  auto LI = Left.rbegin(), LE = Left.rend(), RI = Right.rbegin();
+  // It's common for 2 location calls to start with the same prefix:
+  // getTypeSourceInfo()->getTypeLoc().getAs<..>().getLocalSourceRange()
+  // getTypeSourceInfo()->getTypeLoc().getLocalSourceRange()
+  // We can use pointer identity to quickly try and skip these cases.
+  for (; LI != LE && *LI == *RI; *LI++, *RI++)
+assert(RI != Right.rend());
+  for (; LI != LE; ++LI, ++RI) {
+// If we haven't returned up to here, both call stacks are the same up the
+// here are the same. If we detect that RI is at the end, that means the
+// rights last item was a SourceLocation/Range accessor. Therefore the
+// preceding item in left should also return a Loc/Range and it should be
+// the last item in its stack.
+assert(RI != Right.rend());
+if (auto Strcmp = (*LI)->name().compare((*RI)->name()))
+  return Strcmp;
+// As both calls are the same, they should both either return by value or
+// both return by pointer.
+assert((*LI)->returnsPointer() == (*RI)->returnsPointer());
+  }
+  // As before but flipped.
+  assert(RI == Right.rend());
+  return 0;
+}
+
 namespace internal {
 bool RangeLessThan::operator()(
 std::pair const ,
@@ -57,15 +96,13 @@
   else if (LHS.first.getEnd() != RHS.first.getEnd())
 return false;
 
-  return LocationCallFormatterCpp::format(*LHS.second) <
- LocationCallFormatterCpp::format(*RHS.second);
+  return compareLocationCall(*LHS.second, *RHS.second) < 0;
 }
 bool RangeLessThan::operator()(
 std::pair const ,
 std::pair const ) const {
   if (LHS.first == RHS.first)
-return LocationCallFormatterCpp::format(*LHS.second) <
-   LocationCallFormatterCpp::format(*RHS.second);
+return compareLocationCall(*LHS.second, *RHS.second) < 0;
   return LHS.first < RHS.first;
 }
 } // namespace internal


Index: clang/lib/Tooling/NodeIntrospection.cpp
===
--- clang/lib/Tooling/NodeIntrospection.cpp
+++ clang/lib/Tooling/NodeIntrospection.cpp
@@ -13,6 +13,9 @@
 #include "clang/Tooling/NodeIntrospection.h"
 
 #include "clang/AST/AST.h"
+#include "llvm/Support/Compiler.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
@@ -40,6 +43,42 @@
   return Result;
 }
 
+/// A 3 way comparison of LHS and RHS based on call names.
+static int compareLocationCall(const LocationCall ,
+   const LocationCall ) {
+  // Expand the call stacks into a vector so we can traverse it in reverse
+  // order.
+  llvm::SmallVector Left, Right;
+  for (const LocationCall *Item =  Item; Item = Item->on())
+Left.push_back(Item);
+  for (const LocationCall *Item =  Item; Item = Item->on())
+Right.push_back(Item);
+
+  auto LI = Left.rbegin(), LE = Left.rend(), RI = Right.rbegin();
+  // It's common for 2 location calls to start with the same prefix:
+  // getTypeSourceInfo()->getTypeLoc().getAs<..>().getLocalSourceRange()
+  // getTypeSourceInfo()->getTypeLoc().getLocalSourceRange()
+  // We can use pointer identity to quickly try and skip these cases.
+  for (; LI != LE && *LI == *RI; *LI++, *RI++)
+assert(RI != Right.rend());
+  for (; LI != LE; ++LI, ++RI) {
+// If we haven't returned up to here, both call stacks are the same up the
+// here are the same. If we detect that RI is at the end, that means the
+// rights last item was a SourceLocation/Range accessor. Therefore the
+// preceding item in left should also return a Loc/Range and it should be
+// the