[PATCH] D157518: Avoid running optimization passes in frontend test

[Matthias Braun via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf8431a0e4008: Avoid running optimization passes in frontend 
test (authored by MatzeB).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157518

Files:
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp

Index: clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
===
--- clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
+++ clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
@@ -1,14 +1,31 @@
-// RUN: %clang_cc1 -O1 -emit-llvm %s -o - -triple=x86_64-linux-gnu | FileCheck -DLIKELY=2000 -DUNLIKELY=1 %s
-// RUN: %clang_cc1 -O1 -emit-llvm %s -triple=x86_64-linux-gnu -mllvm -likely-branch-weight=99 -mllvm -unlikely-branch-weight=42 -o - | FileCheck -DLIKELY=99 -DUNLIKELY=42 %s
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -O1 -disable-llvm-passes -emit-llvm %s -o - -triple=x86_64-- | FileCheck %s
 
 extern volatile bool b;
 extern volatile int i;
 extern bool A();
 extern bool B();
 
+// CHECK-LABEL: @_Z1fv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[RETVAL:%.*]] = alloca i1, align 1
+// CHECK-NEXT:[[TMP0:%.*]] = load volatile i8, ptr @b, align 1, !tbaa [[TBAA2:![0-9]+]], !range [[RNG6:![0-9]+]]
+// CHECK-NEXT:[[TOBOOL:%.*]] = trunc i8 [[TMP0]] to i1
+// CHECK-NEXT:[[TOBOOL_EXPVAL:%.*]] = call i1 @llvm.expect.i1(i1 [[TOBOOL]], i1 true)
+// CHECK-NEXT:br i1 [[TOBOOL_EXPVAL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+// CHECK:   if.then:
+// CHECK-NEXT:[[CALL:%.*]] = call noundef zeroext i1 @_Z1Av()
+// CHECK-NEXT:store i1 [[CALL]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN:%.*]]
+// CHECK:   if.end:
+// CHECK-NEXT:[[CALL1:%.*]] = call noundef zeroext i1 @_Z1Bv()
+// CHECK-NEXT:store i1 [[CALL1]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN]]
+// CHECK:   return:
+// CHECK-NEXT:[[TMP1:%.*]] = load i1, ptr [[RETVAL]], align 1
+// CHECK-NEXT:ret i1 [[TMP1]]
+//
 bool f() {
-  // CHECK-LABEL: define{{.*}} zeroext i1 @_Z1fv
-  // CHECK: br {{.*}} !prof ![[PROF_LIKELY:[0-9]+]]
   if (b)
 [[likely]] {
   return A();
@@ -16,9 +33,26 @@
   return B();
 }
 
+// CHECK-LABEL: @_Z1gv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[RETVAL:%.*]] = alloca i1, align 1
+// CHECK-NEXT:[[TMP0:%.*]] = load volatile i8, ptr @b, align 1, !tbaa [[TBAA2]], !range [[RNG6]]
+// CHECK-NEXT:[[TOBOOL:%.*]] = trunc i8 [[TMP0]] to i1
+// CHECK-NEXT:[[TOBOOL_EXPVAL:%.*]] = call i1 @llvm.expect.i1(i1 [[TOBOOL]], i1 false)
+// CHECK-NEXT:br i1 [[TOBOOL_EXPVAL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+// CHECK:   if.then:
+// CHECK-NEXT:[[CALL:%.*]] = call noundef zeroext i1 @_Z1Av()
+// CHECK-NEXT:store i1 [[CALL]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN:%.*]]
+// CHECK:   if.end:
+// CHECK-NEXT:[[CALL1:%.*]] = call noundef zeroext i1 @_Z1Bv()
+// CHECK-NEXT:store i1 [[CALL1]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN]]
+// CHECK:   return:
+// CHECK-NEXT:[[TMP1:%.*]] = load i1, ptr [[RETVAL]], align 1
+// CHECK-NEXT:ret i1 [[TMP1]]
+//
 bool g() {
-  // CHECK-LABEL: define{{.*}} zeroext i1 @_Z1gv
-  // CHECK: br {{.*}} !prof ![[PROF_UNLIKELY:[0-9]+]]
   if (b)
 [[unlikely]] {
   return A();
@@ -27,18 +61,47 @@
   return B();
 }
 
+// CHECK-LABEL: @_Z1hv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[RETVAL:%.*]] = alloca i1, align 1
+// CHECK-NEXT:[[TMP0:%.*]] = load volatile i8, ptr @b, align 1, !tbaa [[TBAA2]], !range [[RNG6]]
+// CHECK-NEXT:[[TOBOOL:%.*]] = trunc i8 [[TMP0]] to i1
+// CHECK-NEXT:[[TOBOOL_EXPVAL:%.*]] = call i1 @llvm.expect.i1(i1 [[TOBOOL]], i1 false)
+// CHECK-NEXT:br i1 [[TOBOOL_EXPVAL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+// CHECK:   if.then:
+// CHECK-NEXT:[[CALL:%.*]] = call noundef zeroext i1 @_Z1Av()
+// CHECK-NEXT:store i1 [[CALL]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN:%.*]]
+// CHECK:   if.end:
+// CHECK-NEXT:[[CALL1:%.*]] = call noundef zeroext i1 @_Z1Bv()
+// CHECK-NEXT:store i1 [[CALL1]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN]]
+// CHECK:   return:
+// CHECK-NEXT:[[TMP1:%.*]] = load i1, ptr [[RETVAL]], align 1
+// CHECK-NEXT:ret i1 [[TMP1]]
+//
 bool h() {
-  // CHECK-LABEL: define{{.*}} zeroext i1 @_Z1hv
-  // CHECK: br {{.*}} !prof ![[PROF_UNLIKELY]]
   if (b)
 [[unlikely]] return A();
 
   return B();
 }
 
+// CHECK-LABEL: @_Z8NullStmtv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = load volatile i8, ptr @b, align 1, !tbaa [[TBAA2]], !range [[RNG6]]
+// CHECK-NEXT:[[TOBOOL:%.*]] = trunc i8 [[TMP0]] to i1
+// CHECK-NEXT:[[TOBOOL_EXPVAL:%.*]] = call i1 

[PATCH] D157518: Avoid running optimization passes in frontend test

[Matthias Braun via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp:147-148
 
-// CHECK: ![[PROF_LIKELY]] = !{!"branch_weights", i32 [[UNLIKELY]], i32 
[[LIKELY]]}
-// CHECK: ![[PROF_UNLIKELY]] = !{!"branch_weights", i32 [[LIKELY]], i32 
[[UNLIKELY]]}

aeubanks wrote:
> wenlei wrote:
> > I thought this is the purpose of the test, which is no longer checked after 
> > change? 
> those should be covered by LowerExpectIntrinsic tests
The frontend rewrites `[[likely]]` and `[[unlikely]]` into calls to 
`llvm.expect`. And this is still tested here. Running `lower-expect-intrinsic` 
pass to lower the `llvm.expect` to `!prof` annotations doesn't need to be 
tested here as we have separate tests for that pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157518

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

[PATCH] D158668: RFC: Add getLikelyBranchWeight helper function

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

I have a feeling @mtrofin would prefer pass-specific weights rather than a 
unified notion of "likely"/"unlikely"... So with the latest round of patches 
it's probably best to abandon this for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158668

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

[PATCH] D158668: RFC: Add getLikelyBranchWeight helper function

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

> I'd also posit, that maybe since we're changing this we should reevaluate the 
> numbers we use as defaults.

Heh, same here. Internally we have a handful of functions that end up using 
`[[likely]]` loop conditions in a triple-nested loops leading to the estimated 
block frequencies going through the roof and unjustly dominating everything 
else in the program... Though admittedly I am not decided yet whether I want to 
blame the 2000:1 ratio or rather the programmers for using the annotation too 
freely.

There is also a disconnect with the GCC documentation saying 
`__attribute__((expected))` corresponding to a 99:1 ratio.

Anyway we probably need a separate/patch discussion to not derail this diff too 
much...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158668

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

[PATCH] D158668: Add getLikelyBranchWeight helper function

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

And as another strawman / discussion-started I put up D158680 
 where I use `!{"branch_weights", i32 1, i32 
0}` to represent likely branches and the actual "LikelyWeight" mostly becomes 
an internal implementation detail of the BranchProbabilityInfo class... This 
seemed like a neat way to get an abstract "likely", "unlikely" notation, but 
not sure of the effects if we no longer would have "truly zero" weights because 
they would be interpreted differently now...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158668

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

[PATCH] D158668: Add getLikelyBranchWeight helper function

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

> On the other hand, I dislike exposing some internal detail of a pass.

I think the question to ask is whether a concept like "likely branch" or 
"unlikely branch" shouldn't be more universal and not be a pass internal 
detail? Otherwise it feels like my patches and other places would need to 
insert new `llvm.expect` usages into the IR and litter the pass pipeline with 
more instances of the LowerExpect pass which seems overkill for little gain to 
me...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158668

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

[PATCH] D158668: Add getLikelyBranchWeight helper function

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

> My initial reaction to this was that we should keep the 
> --unlikely-branch-weights flag available

I don't feel strongly about it and can put it back. But can you give some 
reasoning? I only see this flag having a real use to express small ratios like 
3:2 which doesn't really seem helpful to express "likely"/"unlikely"...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158668

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

[PATCH] D158668: Add getLikelyBranchWeight helper function

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

Putting this out as a strawman/RFC. Should we add this function to allow LLVM 
passes to construct "likely"/"unlikely" branch_weights metadata? (I need this 
in D158642  and D157462 
)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158668

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

[PATCH] D158668: Add getLikelyBranchWeight helper function

[Matthias Braun via Phabricator via cfe-commits]

MatzeB created this revision.
MatzeB added reviewers: lebedev.ri, spatel, davidxl, wenlei, hoy, paulkirth.
Herald added subscribers: StephenFan, modimo, hiraditya, mcrosier.
Herald added a project: All.
MatzeB requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, wangpc.
Herald added projects: clang, LLVM.

- Add getLikelyBranchWeight() function returning the value of the 
`-likely-branch-weight` option defaulting to 2000.
- Remove `-unlikely-branch-weight` option and just use 1.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158668

Files:
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  llvm/docs/MisExpect.rst
  llvm/include/llvm/IR/ProfDataUtils.h
  llvm/lib/IR/ProfDataUtils.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-overflow.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-branch-overflow.ll
===
--- llvm/test/Transforms/PGOProfile/misexpect-branch-overflow.ll
+++ llvm/test/Transforms/PGOProfile/misexpect-branch-overflow.ll
@@ -2,7 +2,7 @@
 ; This can happen when the sum of all counters exceeds the max size of uint32_t
 
 ; RUN: llvm-profdata merge %S/Inputs/misexpect-branch-overflow.proftext -o %t.profdata
-; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -unlikely-branch-weight=2147483648 -likely-branch-weight=2147483648 -S 2>&1 | FileCheck %s --check-prefix=OVERFLOW
+; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=OVERFLOW
 
 ; OVERFLOW-NOT: warning: misexpect-branch.c:22:0: 50.00%
 ; OVERFLOW-NOT: remark: misexpect-branch.c:22:0: Potential performance regression from use of the llvm.expect intrinsic: Annotation was correct on 50.00% (2147483648 / 4294967296) of profiled executions.
@@ -32,9 +32,8 @@
   %lnot1 = xor i1 %lnot, true, !dbg !15
   %lnot.ext = zext i1 %lnot1 to i32, !dbg !15
   %conv = sext i32 %lnot.ext to i64, !dbg !15
-  %expval = call i64 @llvm.expect.i64(i64 %conv, i64 1), !dbg !15
-  %tobool = icmp ne i64 %expval, 0, !dbg !15
-  br i1 %tobool, label %if.then, label %if.else, !dbg !15
+  %tobool = icmp ne i64 %conv, 0, !dbg !15
+  br i1 %tobool, label %if.then, label %if.else, !dbg !15, !prof !21
 
 if.then:  ; preds = %entry
   %1 = load i32, ptr %rando, align 4, !dbg !16, !tbaa !10
@@ -100,3 +99,4 @@
 !18 = !DILocation(line: 25, scope: !6)
 !19 = !DILocation(line: 27, scope: !6)
 !20 = !DILocation(line: 28, scope: !6)
+!21 = !{!"branch_weights", i32 2147483648, i32 2147483648}
Index: llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
===
--- llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
+++ llvm/test/Transforms/LowerExpectIntrinsic/phi_unexpect.ll
@@ -1,4 +1,4 @@
-; RUN: opt -S -passes='function(lower-expect),strip-dead-prototypes' -likely-branch-weight=2147483647 -unlikely-branch-weight=1 < %s | FileCheck %s
+; RUN: opt -S -passes='function(lower-expect),strip-dead-prototypes' -likely-branch-weight=2147483647 < %s | FileCheck %s
 
 ; The C case
 ; if (__builtin_expect_with_probability(((a0 == 1) || (a1 == 1) || (a2 == 1)), 1, 0))
Index: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
===
--- llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
+++ llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
@@ -21,6 +21,7 @@
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
+#include "llvm/IR/ProfDataUtils.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/CommandLine.h"
@@ -36,31 +37,11 @@
 STATISTIC(ExpectIntrinsicsHandled,
   "Number of 'expect' intrinsic instructions handled");
 
-// These default values are chosen to represent an extremely skewed outcome for
-// a condition, but they leave some room for interpretation by later passes.
-//
-// If the documentation for __builtin_expect() was made explicit that it should
-// only be used in extreme cases, we could make this ratio higher. As it stands,
-// programmers may be using __builtin_expect() / llvm.expect to annotate that a
-// branch is likely or unlikely to be taken.
-
-// WARNING: these values are internal implementation detail of the pass.
-// They should not be exposed to the outside of the pass, front-end codegen
-// should emit @llvm.expect intrinsics instead of using these weights directly.
-// Transforms should use TargetTransformInfo's getPredictableBranchThreshold().
-static cl::opt LikelyBranchWeight(
-"likely-branch-weight", cl::Hidden, cl::init(2000),
-cl::desc("Weight of the 

[PATCH] D157518: Avoid running optimization passes in frontend test

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 548657.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157518

Files:
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp

Index: clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
===
--- clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
+++ clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
@@ -1,14 +1,31 @@
-// RUN: %clang_cc1 -O1 -emit-llvm %s -o - -triple=x86_64-linux-gnu | FileCheck -DLIKELY=2000 -DUNLIKELY=1 %s
-// RUN: %clang_cc1 -O1 -emit-llvm %s -triple=x86_64-linux-gnu -mllvm -likely-branch-weight=99 -mllvm -unlikely-branch-weight=42 -o - | FileCheck -DLIKELY=99 -DUNLIKELY=42 %s
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -O1 -disable-llvm-passes -emit-llvm %s -o - -triple=x86_64-- | FileCheck %s
 
 extern volatile bool b;
 extern volatile int i;
 extern bool A();
 extern bool B();
 
+// CHECK-LABEL: @_Z1fv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[RETVAL:%.*]] = alloca i1, align 1
+// CHECK-NEXT:[[TMP0:%.*]] = load volatile i8, ptr @b, align 1, !tbaa [[TBAA2:![0-9]+]], !range [[RNG6:![0-9]+]]
+// CHECK-NEXT:[[TOBOOL:%.*]] = trunc i8 [[TMP0]] to i1
+// CHECK-NEXT:[[TOBOOL_EXPVAL:%.*]] = call i1 @llvm.expect.i1(i1 [[TOBOOL]], i1 true)
+// CHECK-NEXT:br i1 [[TOBOOL_EXPVAL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+// CHECK:   if.then:
+// CHECK-NEXT:[[CALL:%.*]] = call noundef zeroext i1 @_Z1Av()
+// CHECK-NEXT:store i1 [[CALL]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN:%.*]]
+// CHECK:   if.end:
+// CHECK-NEXT:[[CALL1:%.*]] = call noundef zeroext i1 @_Z1Bv()
+// CHECK-NEXT:store i1 [[CALL1]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN]]
+// CHECK:   return:
+// CHECK-NEXT:[[TMP1:%.*]] = load i1, ptr [[RETVAL]], align 1
+// CHECK-NEXT:ret i1 [[TMP1]]
+//
 bool f() {
-  // CHECK-LABEL: define{{.*}} zeroext i1 @_Z1fv
-  // CHECK: br {{.*}} !prof ![[PROF_LIKELY:[0-9]+]]
   if (b)
 [[likely]] {
   return A();
@@ -16,9 +33,26 @@
   return B();
 }
 
+// CHECK-LABEL: @_Z1gv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[RETVAL:%.*]] = alloca i1, align 1
+// CHECK-NEXT:[[TMP0:%.*]] = load volatile i8, ptr @b, align 1, !tbaa [[TBAA2]], !range [[RNG6]]
+// CHECK-NEXT:[[TOBOOL:%.*]] = trunc i8 [[TMP0]] to i1
+// CHECK-NEXT:[[TOBOOL_EXPVAL:%.*]] = call i1 @llvm.expect.i1(i1 [[TOBOOL]], i1 false)
+// CHECK-NEXT:br i1 [[TOBOOL_EXPVAL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+// CHECK:   if.then:
+// CHECK-NEXT:[[CALL:%.*]] = call noundef zeroext i1 @_Z1Av()
+// CHECK-NEXT:store i1 [[CALL]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN:%.*]]
+// CHECK:   if.end:
+// CHECK-NEXT:[[CALL1:%.*]] = call noundef zeroext i1 @_Z1Bv()
+// CHECK-NEXT:store i1 [[CALL1]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN]]
+// CHECK:   return:
+// CHECK-NEXT:[[TMP1:%.*]] = load i1, ptr [[RETVAL]], align 1
+// CHECK-NEXT:ret i1 [[TMP1]]
+//
 bool g() {
-  // CHECK-LABEL: define{{.*}} zeroext i1 @_Z1gv
-  // CHECK: br {{.*}} !prof ![[PROF_UNLIKELY:[0-9]+]]
   if (b)
 [[unlikely]] {
   return A();
@@ -27,18 +61,47 @@
   return B();
 }
 
+// CHECK-LABEL: @_Z1hv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[RETVAL:%.*]] = alloca i1, align 1
+// CHECK-NEXT:[[TMP0:%.*]] = load volatile i8, ptr @b, align 1, !tbaa [[TBAA2]], !range [[RNG6]]
+// CHECK-NEXT:[[TOBOOL:%.*]] = trunc i8 [[TMP0]] to i1
+// CHECK-NEXT:[[TOBOOL_EXPVAL:%.*]] = call i1 @llvm.expect.i1(i1 [[TOBOOL]], i1 false)
+// CHECK-NEXT:br i1 [[TOBOOL_EXPVAL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+// CHECK:   if.then:
+// CHECK-NEXT:[[CALL:%.*]] = call noundef zeroext i1 @_Z1Av()
+// CHECK-NEXT:store i1 [[CALL]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN:%.*]]
+// CHECK:   if.end:
+// CHECK-NEXT:[[CALL1:%.*]] = call noundef zeroext i1 @_Z1Bv()
+// CHECK-NEXT:store i1 [[CALL1]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN]]
+// CHECK:   return:
+// CHECK-NEXT:[[TMP1:%.*]] = load i1, ptr [[RETVAL]], align 1
+// CHECK-NEXT:ret i1 [[TMP1]]
+//
 bool h() {
-  // CHECK-LABEL: define{{.*}} zeroext i1 @_Z1hv
-  // CHECK: br {{.*}} !prof ![[PROF_UNLIKELY]]
   if (b)
 [[unlikely]] return A();
 
   return B();
 }
 
+// CHECK-LABEL: @_Z8NullStmtv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = load volatile i8, ptr @b, align 1, !tbaa [[TBAA2]], !range [[RNG6]]
+// CHECK-NEXT:[[TOBOOL:%.*]] = trunc i8 [[TMP0]] to i1
+// CHECK-NEXT:[[TOBOOL_EXPVAL:%.*]] = call i1 @llvm.expect.i1(i1 [[TOBOOL]], i1 false)
+// CHECK-NEXT:br i1 [[TOBOOL_EXPVAL]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+// 

[PATCH] D157518: Avoid running optimization passes in frontend test

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 548655.
MatzeB added a comment.

Decided to just use "utils/update_cc_test_checks.py" so we can inspect the 
generate llvm.expect calls directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157518

Files:
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp

Index: clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
===
--- clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
+++ clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
@@ -1,14 +1,31 @@
-// RUN: %clang_cc1 -O1 -emit-llvm %s -o - -triple=x86_64-linux-gnu | FileCheck -DLIKELY=2000 -DUNLIKELY=1 %s
-// RUN: %clang_cc1 -O1 -emit-llvm %s -triple=x86_64-linux-gnu -mllvm -likely-branch-weight=99 -mllvm -unlikely-branch-weight=42 -o - | FileCheck -DLIKELY=99 -DUNLIKELY=42 %s
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -O1 -disable-llvm-passes -emit-llvm %s -o - -triple=x86_64-linux-gnu | FileCheck -DLIKELY=2000 -DUNLIKELY=1 %s
 
 extern volatile bool b;
 extern volatile int i;
 extern bool A();
 extern bool B();
 
+// CHECK-LABEL: @_Z1fv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[RETVAL:%.*]] = alloca i1, align 1
+// CHECK-NEXT:[[TMP0:%.*]] = load volatile i8, ptr @b, align 1, !tbaa [[TBAA2:![0-9]+]], !range [[RNG6:![0-9]+]]
+// CHECK-NEXT:[[TOBOOL:%.*]] = trunc i8 [[TMP0]] to i1
+// CHECK-NEXT:[[TOBOOL_EXPVAL:%.*]] = call i1 @llvm.expect.i1(i1 [[TOBOOL]], i1 true)
+// CHECK-NEXT:br i1 [[TOBOOL_EXPVAL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+// CHECK:   if.then:
+// CHECK-NEXT:[[CALL:%.*]] = call noundef zeroext i1 @_Z1Av()
+// CHECK-NEXT:store i1 [[CALL]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN:%.*]]
+// CHECK:   if.end:
+// CHECK-NEXT:[[CALL1:%.*]] = call noundef zeroext i1 @_Z1Bv()
+// CHECK-NEXT:store i1 [[CALL1]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN]]
+// CHECK:   return:
+// CHECK-NEXT:[[TMP1:%.*]] = load i1, ptr [[RETVAL]], align 1
+// CHECK-NEXT:ret i1 [[TMP1]]
+//
 bool f() {
-  // CHECK-LABEL: define{{.*}} zeroext i1 @_Z1fv
-  // CHECK: br {{.*}} !prof ![[PROF_LIKELY:[0-9]+]]
   if (b)
 [[likely]] {
   return A();
@@ -16,9 +33,26 @@
   return B();
 }
 
+// CHECK-LABEL: @_Z1gv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[RETVAL:%.*]] = alloca i1, align 1
+// CHECK-NEXT:[[TMP0:%.*]] = load volatile i8, ptr @b, align 1, !tbaa [[TBAA2]], !range [[RNG6]]
+// CHECK-NEXT:[[TOBOOL:%.*]] = trunc i8 [[TMP0]] to i1
+// CHECK-NEXT:[[TOBOOL_EXPVAL:%.*]] = call i1 @llvm.expect.i1(i1 [[TOBOOL]], i1 false)
+// CHECK-NEXT:br i1 [[TOBOOL_EXPVAL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+// CHECK:   if.then:
+// CHECK-NEXT:[[CALL:%.*]] = call noundef zeroext i1 @_Z1Av()
+// CHECK-NEXT:store i1 [[CALL]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN:%.*]]
+// CHECK:   if.end:
+// CHECK-NEXT:[[CALL1:%.*]] = call noundef zeroext i1 @_Z1Bv()
+// CHECK-NEXT:store i1 [[CALL1]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN]]
+// CHECK:   return:
+// CHECK-NEXT:[[TMP1:%.*]] = load i1, ptr [[RETVAL]], align 1
+// CHECK-NEXT:ret i1 [[TMP1]]
+//
 bool g() {
-  // CHECK-LABEL: define{{.*}} zeroext i1 @_Z1gv
-  // CHECK: br {{.*}} !prof ![[PROF_UNLIKELY:[0-9]+]]
   if (b)
 [[unlikely]] {
   return A();
@@ -27,18 +61,47 @@
   return B();
 }
 
+// CHECK-LABEL: @_Z1hv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[RETVAL:%.*]] = alloca i1, align 1
+// CHECK-NEXT:[[TMP0:%.*]] = load volatile i8, ptr @b, align 1, !tbaa [[TBAA2]], !range [[RNG6]]
+// CHECK-NEXT:[[TOBOOL:%.*]] = trunc i8 [[TMP0]] to i1
+// CHECK-NEXT:[[TOBOOL_EXPVAL:%.*]] = call i1 @llvm.expect.i1(i1 [[TOBOOL]], i1 false)
+// CHECK-NEXT:br i1 [[TOBOOL_EXPVAL]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
+// CHECK:   if.then:
+// CHECK-NEXT:[[CALL:%.*]] = call noundef zeroext i1 @_Z1Av()
+// CHECK-NEXT:store i1 [[CALL]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN:%.*]]
+// CHECK:   if.end:
+// CHECK-NEXT:[[CALL1:%.*]] = call noundef zeroext i1 @_Z1Bv()
+// CHECK-NEXT:store i1 [[CALL1]], ptr [[RETVAL]], align 1
+// CHECK-NEXT:br label [[RETURN]]
+// CHECK:   return:
+// CHECK-NEXT:[[TMP1:%.*]] = load i1, ptr [[RETVAL]], align 1
+// CHECK-NEXT:ret i1 [[TMP1]]
+//
 bool h() {
-  // CHECK-LABEL: define{{.*}} zeroext i1 @_Z1hv
-  // CHECK: br {{.*}} !prof ![[PROF_UNLIKELY]]
   if (b)
 [[unlikely]] return A();
 
   return B();
 }
 
+// CHECK-LABEL: @_Z8NullStmtv(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = load volatile i8, ptr @b, align 1, !tbaa [[TBAA2]], !range [[RNG6]]
+// CHECK-NEXT:[[TOBOOL:%.*]] = trunc i8 [[TMP0]] to i1
+// CHECK-NEXT:  

[PATCH] D157518: Avoid running optimization passes in frontend test

[Matthias Braun via Phabricator via cfe-commits]

MatzeB created this revision.
MatzeB added reviewers: wenlei, Mordante, aeubanks, hoy.
Herald added subscribers: modimo, mcrosier.
Herald added a project: All.
MatzeB requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Only run "lower-expect" but no other optimization passes in test for 
likely/unlikely annotations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157518

Files:
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp


Index: clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
===
--- clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
+++ clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -O1 -emit-llvm %s -o - -triple=x86_64-linux-gnu | FileCheck 
-DLIKELY=2000 -DUNLIKELY=1 %s
-// RUN: %clang_cc1 -O1 -emit-llvm %s -triple=x86_64-linux-gnu -mllvm 
-likely-branch-weight=99 -mllvm -unlikely-branch-weight=42 -o - | FileCheck 
-DLIKELY=99 -DUNLIKELY=42 %s
+// RUN: %clang_cc1 -O1 -disable-llvm-passes -emit-llvm %s -o - 
-triple=x86_64-linux-gnu | opt -S -passes=lower-expect | FileCheck 
-DLIKELY=2000 -DUNLIKELY=1 %s
+// RUN: %clang_cc1 -O1 -disable-llvm-passes -emit-llvm %s 
-triple=x86_64-linux-gnu -o - | opt -S -passes=lower-expect 
-likely-branch-weight=99 -unlikely-branch-weight=42 | FileCheck -DLIKELY=99 
-DUNLIKELY=42 %s
 
 extern volatile bool b;
 extern volatile int i;
@@ -69,7 +69,7 @@
 
   // CHECK-NOT: br {{.*}} %if.end{{.*}} !prof
   if (b)
-// CHECK: br {{.*}} !prof ![[PROF_LIKELY]]
+// CHECK: br {{.*}} !prof ![[PROF_UNLIKELY]]
 while (B())
   [[unlikely]] { b = false; }
 }
@@ -97,7 +97,7 @@
 
   // CHECK-NOT: br {{.*}} %if.end{{.*}} !prof
   if (b)
-// CHECK: br {{.*}} !prof ![[PROF_LIKELY]]
+// CHECK: br {{.*}} !prof ![[PROF_UNLIKELY]]
 for (; B();)
   [[unlikely]] {}
 }
@@ -144,5 +144,5 @@
   }
 }
 
-// CHECK: ![[PROF_LIKELY]] = !{!"branch_weights", i32 [[UNLIKELY]], i32 
[[LIKELY]]}
-// CHECK: ![[PROF_UNLIKELY]] = !{!"branch_weights", i32 [[LIKELY]], i32 
[[UNLIKELY]]}
+// CHECK: ![[PROF_LIKELY]] = !{!"branch_weights", i32 [[LIKELY]], i32 
[[UNLIKELY]]}
+// CHECK: ![[PROF_UNLIKELY]] = !{!"branch_weights", i32 [[UNLIKELY]], i32 
[[LIKELY]]}


Index: clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
===
--- clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
+++ clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -O1 -emit-llvm %s -o - -triple=x86_64-linux-gnu | FileCheck -DLIKELY=2000 -DUNLIKELY=1 %s
-// RUN: %clang_cc1 -O1 -emit-llvm %s -triple=x86_64-linux-gnu -mllvm -likely-branch-weight=99 -mllvm -unlikely-branch-weight=42 -o - | FileCheck -DLIKELY=99 -DUNLIKELY=42 %s
+// RUN: %clang_cc1 -O1 -disable-llvm-passes -emit-llvm %s -o - -triple=x86_64-linux-gnu | opt -S -passes=lower-expect | FileCheck -DLIKELY=2000 -DUNLIKELY=1 %s
+// RUN: %clang_cc1 -O1 -disable-llvm-passes -emit-llvm %s -triple=x86_64-linux-gnu -o - | opt -S -passes=lower-expect -likely-branch-weight=99 -unlikely-branch-weight=42 | FileCheck -DLIKELY=99 -DUNLIKELY=42 %s
 
 extern volatile bool b;
 extern volatile int i;
@@ -69,7 +69,7 @@
 
   // CHECK-NOT: br {{.*}} %if.end{{.*}} !prof
   if (b)
-// CHECK: br {{.*}} !prof ![[PROF_LIKELY]]
+// CHECK: br {{.*}} !prof ![[PROF_UNLIKELY]]
 while (B())
   [[unlikely]] { b = false; }
 }
@@ -97,7 +97,7 @@
 
   // CHECK-NOT: br {{.*}} %if.end{{.*}} !prof
   if (b)
-// CHECK: br {{.*}} !prof ![[PROF_LIKELY]]
+// CHECK: br {{.*}} !prof ![[PROF_UNLIKELY]]
 for (; B();)
   [[unlikely]] {}
 }
@@ -144,5 +144,5 @@
   }
 }
 
-// CHECK: ![[PROF_LIKELY]] = !{!"branch_weights", i32 [[UNLIKELY]], i32 [[LIKELY]]}
-// CHECK: ![[PROF_UNLIKELY]] = !{!"branch_weights", i32 [[LIKELY]], i32 [[UNLIKELY]]}
+// CHECK: ![[PROF_LIKELY]] = !{!"branch_weights", i32 [[LIKELY]], i32 [[UNLIKELY]]}
+// CHECK: ![[PROF_UNLIKELY]] = !{!"branch_weights", i32 [[UNLIKELY]], i32 [[LIKELY]]}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

In D154658#4533476 , @MatzeB wrote:

> This change results in some of our builds (distributed Thin-LTO in case that 
> matters) to fail with missing symbols. At a first glance this seems to emit 
> VTables in some files where it didn't do this before and then fails to 
> resolve some members of that vtable. I'm in the process of analyzing this 
> further and making a small reproducer.

I just noticed b6847edfc235829b37dd6d734ef5bbfa0a58b6fc 
 mentioned 
in the task above which isn't part of our builds yet. I see even symbols 
emitted with that change and suspect it may resolves the undefined symbols 
problems then. Will know for sure in a day or two (and if you hear nothing all 
is working well).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154658

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

[PATCH] D154658: Optimize emission of `dynamic_cast` to final classes.

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

This change results in some of our builds (distributed Thin-LTO in case that 
matters) to fail with missing symbols. At a first glance this seems to emit 
VTables in some files where it didn't do this before and then fails to resolve 
some members of that vtable. I'm in the process of analyzing this further and 
making a small reproducer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154658

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

[PATCH] D150761: [NFC][Py Reformat] Reformat python files in clang and clang-tools-extra

[Matthias Braun via Phabricator via cfe-commits]

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

Inspected 5 random files and they looked fine as expected. Do you know if the 
test failures are unrealted?

LGTM with test failures resolved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150761

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

[PATCH] D146758: Fix codegen for coroutine with function-try-block

[Matthias Braun 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 rGe00a8d081d78: Fix codegen for coroutine with 
function-try-block (authored by MatzeB).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146758

Files:
  clang/include/clang/AST/StmtCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/StmtCXX.cpp
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CodeGenCoroutines/coro-function-try-block.cpp

Index: clang/test/CodeGenCoroutines/coro-function-try-block.cpp
===
--- /dev/null
+++ clang/test/CodeGenCoroutines/coro-function-try-block.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \
+// RUN:-disable-llvm-passes %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct task {
+  struct promise_type {
+task get_return_object();
+std::suspend_never initial_suspend();
+std::suspend_never final_suspend() noexcept;
+void return_void();
+void unhandled_exception() noexcept;
+  };
+};
+
+task f() try {
+  co_return;
+} catch(...) {
+}
+
+// CHECK-LABEL: define{{.*}} void @_Z1fv(
+// CHECK: call void @_ZNSt13suspend_never13await_suspendESt16coroutine_handleIvE(
+// CHECK: call void @_ZN4task12promise_type11return_voidEv(
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -4546,7 +4546,8 @@
 
   FSI->setHasCXXTry(TryLoc);
 
-  return CXXTryStmt::Create(Context, TryLoc, TryBlock, Handlers);
+  return CXXTryStmt::Create(Context, TryLoc, cast(TryBlock),
+Handlers);
 }
 
 StmtResult Sema::ActOnSEHTryBlock(bool IsCXXTry, SourceLocation TryLoc,
Index: clang/lib/CodeGen/CGCoroutine.cpp
===
--- clang/lib/CodeGen/CGCoroutine.cpp
+++ clang/lib/CodeGen/CGCoroutine.cpp
@@ -593,6 +593,18 @@
   CGF.EmitStmt(OnFallthrough);
 }
 
+static CompoundStmt *CoroutineStmtBuilder(ASTContext ,
+  const CoroutineBodyStmt ) {
+  Stmt *Stmt = S.getBody();
+  if (CompoundStmt *Body = dyn_cast(Stmt))
+return Body;
+  // We are about to create a `CXXTryStmt` which requires a `CompoundStmt`.
+  // If the function body is not a `CompoundStmt` yet then we have to create
+  // a new one. This happens for cases like the "function-try-block" syntax.
+  return CompoundStmt::Create(Context, {Stmt}, FPOptionsOverride(),
+  SourceLocation(), SourceLocation());
+}
+
 void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt ) {
   auto *NullPtr = llvm::ConstantPointerNull::get(Builder.getInt8PtrTy());
   auto  = CGM.getContext().getTargetInfo();
@@ -721,8 +733,8 @@
   auto Loc = S.getBeginLoc();
   CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr,
  CurCoro.Data->ExceptionHandler);
-  auto *TryStmt =
-  CXXTryStmt::Create(getContext(), Loc, S.getBody(), );
+  CompoundStmt *Body = CoroutineStmtBuilder(getContext(), S);
+  auto *TryStmt = CXXTryStmt::Create(getContext(), Loc, Body, );
 
   EnterCXXTryStmt(*TryStmt);
   emitBodyAndFallthrough(*this, S, TryStmt->getTryBlock());
Index: clang/lib/AST/StmtCXX.cpp
===
--- clang/lib/AST/StmtCXX.cpp
+++ clang/lib/AST/StmtCXX.cpp
@@ -23,7 +23,8 @@
 }
 
 CXXTryStmt *CXXTryStmt::Create(const ASTContext , SourceLocation tryLoc,
-   Stmt *tryBlock, ArrayRef handlers) {
+   CompoundStmt *tryBlock,
+   ArrayRef handlers) {
   const size_t Size = totalSizeToAlloc(handlers.size() + 1);
   void *Mem = C.Allocate(Size, alignof(CXXTryStmt));
   return new (Mem) CXXTryStmt(tryLoc, tryBlock, handlers);
@@ -36,7 +37,7 @@
   return new (Mem) CXXTryStmt(Empty, numHandlers);
 }
 
-CXXTryStmt::CXXTryStmt(SourceLocation tryLoc, Stmt *tryBlock,
+CXXTryStmt::CXXTryStmt(SourceLocation tryLoc, CompoundStmt *tryBlock,
ArrayRef handlers)
 : Stmt(CXXTryStmtClass), TryLoc(tryLoc), NumHandlers(handlers.size()) {
   Stmt **Stmts = getStmts();
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -6793,8 +6793,8 @@
   return ToHandlerOrErr.takeError();
   }
 
-  return CXXTryStmt::Create(
-  Importer.getToContext(), *ToTryLocOrErr,*ToTryBlockOrErr, ToHandlers);
+  return CXXTryStmt::Create(Importer.getToContext(), *ToTryLocOrErr,
+cast(*ToTryBlockOrErr), ToHandlers);
 }
 
 ExpectedStmt 

[PATCH] D146758: Fix codegen for coroutine with function-try-block

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added inline comments.



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:724-730
+  Stmt *BodyStmt = S.getBody();
+  CompoundStmt *Body = dyn_cast(BodyStmt);
+  if (Body == nullptr) {
+Body =
+CompoundStmt::Create(getContext(), {BodyStmt}, FPOptionsOverride(),
+ SourceLocation(), SourceLocation());
+  }

ChuanqiXu wrote:
> MatzeB wrote:
> > ChuanqiXu wrote:
> > > MatzeB wrote:
> > > > ChuanqiXu wrote:
> > > > > Can we try to move the logic to `CoroutineStmtBuilder`? That makes me 
> > > > > feel better. And it will be helpful to add a comment to tell that 
> > > > > we're handling the case the function body is function-try-block.
> > > > I'll add a detailed comment. But would you be fine leaving the 
> > > > statements here as-is? The logic only makes sense in the context of 
> > > > using the `Body` to create a `CXXTryStmt` below (it's really an effect 
> > > > of `CXXTryStmt` only accepting CompountStmt operands).
> > > It looks like you didn't address the comments. Would you like to address 
> > > it? I don't mind to address it later myself.
> > Did you mean to create a new function named `CoroutineStmtBuilder` like I 
> > did now?
> > Did you mean to create a new function named CoroutineStmtBuilder like I did 
> > now?
> 
> No, I mean we should construct this in Sema.
> 
> > Putting an assert here feels unnecessary and may be in the way if in the 
> > future we ever allow other types of single-statement function bodies.
> 
> Personally I prefer the more precise style.
> No, I mean we should construct this in Sema.

I'll land the patch as-is then and leave the refactoring to you if necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146758

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

[PATCH] D146758: Fix codegen for coroutine with function-try-block

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added inline comments.



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:724-730
+  Stmt *BodyStmt = S.getBody();
+  CompoundStmt *Body = dyn_cast(BodyStmt);
+  if (Body == nullptr) {
+Body =
+CompoundStmt::Create(getContext(), {BodyStmt}, FPOptionsOverride(),
+ SourceLocation(), SourceLocation());
+  }

ChuanqiXu wrote:
> Can we try to move the logic to `CoroutineStmtBuilder`? That makes me feel 
> better. And it will be helpful to add a comment to tell that we're handling 
> the case the function body is function-try-block.
I'll add a detailed comment. But would you be fine leaving the statements here 
as-is? The logic only makes sense in the context of using the `Body` to create 
a `CXXTryStmt` below (it's really an effect of `CXXTryStmt` only accepting 
CompountStmt operands).



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:724-730
+  Stmt *BodyStmt = S.getBody();
+  CompoundStmt *Body = dyn_cast(BodyStmt);
+  if (Body == nullptr) {
+Body =
+CompoundStmt::Create(getContext(), {BodyStmt}, FPOptionsOverride(),
+ SourceLocation(), SourceLocation());
+  }

ChuanqiXu wrote:
> MatzeB wrote:
> > ChuanqiXu wrote:
> > > Can we try to move the logic to `CoroutineStmtBuilder`? That makes me 
> > > feel better. And it will be helpful to add a comment to tell that we're 
> > > handling the case the function body is function-try-block.
> > I'll add a detailed comment. But would you be fine leaving the statements 
> > here as-is? The logic only makes sense in the context of using the `Body` 
> > to create a `CXXTryStmt` below (it's really an effect of `CXXTryStmt` only 
> > accepting CompountStmt operands).
> It looks like you didn't address the comments. Would you like to address it? 
> I don't mind to address it later myself.
Did you mean to create a new function named `CoroutineStmtBuilder` like I did 
now?



Comment at: clang/lib/CodeGen/CGCoroutine.cpp:726
+  CompoundStmt *Body = dyn_cast(BodyStmt);
+  if (Body == nullptr) {
+Body =

ChuanqiXu wrote:
> It reads better to specify the potential type for Body.
I will mention a single-statement body as an example in the comment now. 
Putting an `assert` here feels unnecessary and may be in the way if in the 
future we ever allow other types of single-statement function bodies.



Comment at: clang/test/CodeGenCoroutines/coro-function-try-block.cpp:21-23
+// CHECK-LABEL: define{{.*}} void @_Z1fv(
+// CHECK: call void 
@_ZNSt13suspend_never13await_suspendESt16coroutine_handleIvE(
+// CHECK: call void @_ZN4task12promise_type11return_voidEv(

ChuanqiXu wrote:
> I expect to see the nested try statements in the case.
This was mostly meant as a test for "does not crash the compiler", so it is 
just checking some parts of the output without caring too much what they are 
specifically.

I cannot test for nested try statements, because in llvm-ir those are already 
lowered to unwind tables and landing pad blocks, but there's neither a "try" 
statement nor an immediate nesting I could test for.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146758

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

[PATCH] D146758: Fix codegen for coroutine with function-try-block

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 509173.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146758

Files:
  clang/include/clang/AST/StmtCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/StmtCXX.cpp
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CodeGenCoroutines/coro-function-try-block.cpp

Index: clang/test/CodeGenCoroutines/coro-function-try-block.cpp
===
--- /dev/null
+++ clang/test/CodeGenCoroutines/coro-function-try-block.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \
+// RUN:-disable-llvm-passes %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct task {
+  struct promise_type {
+task get_return_object();
+std::suspend_never initial_suspend();
+std::suspend_never final_suspend() noexcept;
+void return_void();
+void unhandled_exception() noexcept;
+  };
+};
+
+task f() try {
+  co_return;
+} catch(...) {
+}
+
+// CHECK-LABEL: define{{.*}} void @_Z1fv(
+// CHECK: call void @_ZNSt13suspend_never13await_suspendESt16coroutine_handleIvE(
+// CHECK: call void @_ZN4task12promise_type11return_voidEv(
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -4546,7 +4546,8 @@
 
   FSI->setHasCXXTry(TryLoc);
 
-  return CXXTryStmt::Create(Context, TryLoc, TryBlock, Handlers);
+  return CXXTryStmt::Create(Context, TryLoc, cast(TryBlock),
+Handlers);
 }
 
 StmtResult Sema::ActOnSEHTryBlock(bool IsCXXTry, SourceLocation TryLoc,
Index: clang/lib/CodeGen/CGCoroutine.cpp
===
--- clang/lib/CodeGen/CGCoroutine.cpp
+++ clang/lib/CodeGen/CGCoroutine.cpp
@@ -593,6 +593,18 @@
   CGF.EmitStmt(OnFallthrough);
 }
 
+static CompoundStmt *CoroutineStmtBuilder(ASTContext ,
+  const CoroutineBodyStmt ) {
+  Stmt *Stmt = S.getBody();
+  if (CompoundStmt *Body = dyn_cast(Stmt))
+return Body;
+  // We are about to create a `CXXTryStmt` which requires a `CompoundStmt`.
+  // If the function body is not a `CompoundStmt` yet then we have to create
+  // a new one. This happens for cases like the "function-try-block" syntax.
+  return CompoundStmt::Create(Context, {Stmt}, FPOptionsOverride(),
+  SourceLocation(), SourceLocation());
+}
+
 void CodeGenFunction::EmitCoroutineBody(const CoroutineBodyStmt ) {
   auto *NullPtr = llvm::ConstantPointerNull::get(Builder.getInt8PtrTy());
   auto  = CGM.getContext().getTargetInfo();
@@ -721,8 +733,8 @@
   auto Loc = S.getBeginLoc();
   CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr,
  CurCoro.Data->ExceptionHandler);
-  auto *TryStmt =
-  CXXTryStmt::Create(getContext(), Loc, S.getBody(), );
+  CompoundStmt *Body = CoroutineStmtBuilder(getContext(), S);
+  auto *TryStmt = CXXTryStmt::Create(getContext(), Loc, Body, );
 
   EnterCXXTryStmt(*TryStmt);
   emitBodyAndFallthrough(*this, S, TryStmt->getTryBlock());
Index: clang/lib/AST/StmtCXX.cpp
===
--- clang/lib/AST/StmtCXX.cpp
+++ clang/lib/AST/StmtCXX.cpp
@@ -23,7 +23,8 @@
 }
 
 CXXTryStmt *CXXTryStmt::Create(const ASTContext , SourceLocation tryLoc,
-   Stmt *tryBlock, ArrayRef handlers) {
+   CompoundStmt *tryBlock,
+   ArrayRef handlers) {
   const size_t Size = totalSizeToAlloc(handlers.size() + 1);
   void *Mem = C.Allocate(Size, alignof(CXXTryStmt));
   return new (Mem) CXXTryStmt(tryLoc, tryBlock, handlers);
@@ -36,7 +37,7 @@
   return new (Mem) CXXTryStmt(Empty, numHandlers);
 }
 
-CXXTryStmt::CXXTryStmt(SourceLocation tryLoc, Stmt *tryBlock,
+CXXTryStmt::CXXTryStmt(SourceLocation tryLoc, CompoundStmt *tryBlock,
ArrayRef handlers)
 : Stmt(CXXTryStmtClass), TryLoc(tryLoc), NumHandlers(handlers.size()) {
   Stmt **Stmts = getStmts();
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -6793,8 +6793,8 @@
   return ToHandlerOrErr.takeError();
   }
 
-  return CXXTryStmt::Create(
-  Importer.getToContext(), *ToTryLocOrErr,*ToTryBlockOrErr, ToHandlers);
+  return CXXTryStmt::Create(Importer.getToContext(), *ToTryLocOrErr,
+cast(*ToTryBlockOrErr), ToHandlers);
 }
 
 ExpectedStmt ASTNodeImporter::VisitCXXForRangeStmt(CXXForRangeStmt *S) {
Index: clang/include/clang/AST/StmtCXX.h
===
--- clang/include/clang/AST/StmtCXX.h
+++ 

[PATCH] D146758: Fix codegen for coroutine with function-try-block

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 508792.
MatzeB marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146758

Files:
  clang/include/clang/AST/StmtCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/StmtCXX.cpp
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CodeGenCoroutines/coro-function-try-block.cpp

Index: clang/test/CodeGenCoroutines/coro-function-try-block.cpp
===
--- /dev/null
+++ clang/test/CodeGenCoroutines/coro-function-try-block.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \
+// RUN:-disable-llvm-passes %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct task {
+  struct promise_type {
+task get_return_object();
+std::suspend_never initial_suspend();
+std::suspend_never final_suspend() noexcept;
+void return_void();
+void unhandled_exception() noexcept;
+  };
+};
+
+task f() try {
+  co_return;
+} catch(...) {
+}
+
+// CHECK-LABEL: define{{.*}} void @_Z1fv(
+// CHECK: call void @_ZNSt13suspend_never13await_suspendESt16coroutine_handleIvE(
+// CHECK: call void @_ZN4task12promise_type11return_voidEv(
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -4546,7 +4546,8 @@
 
   FSI->setHasCXXTry(TryLoc);
 
-  return CXXTryStmt::Create(Context, TryLoc, TryBlock, Handlers);
+  return CXXTryStmt::Create(Context, TryLoc, cast(TryBlock),
+Handlers);
 }
 
 StmtResult Sema::ActOnSEHTryBlock(bool IsCXXTry, SourceLocation TryLoc,
Index: clang/lib/CodeGen/CGCoroutine.cpp
===
--- clang/lib/CodeGen/CGCoroutine.cpp
+++ clang/lib/CodeGen/CGCoroutine.cpp
@@ -721,8 +721,17 @@
   auto Loc = S.getBeginLoc();
   CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr,
  CurCoro.Data->ExceptionHandler);
-  auto *TryStmt =
-  CXXTryStmt::Create(getContext(), Loc, S.getBody(), );
+  Stmt *BodyStmt = S.getBody();
+  // We are about to create a `CXXTryStmt` which requires a `CompoundStmt`.
+  // We have to create a new CompoundStmt if the function body is not a
+  // `CompoundStmt` yet in cases like the "function-try-block" syntax.
+  CompoundStmt *Body = dyn_cast(BodyStmt);
+  if (Body == nullptr) {
+Body =
+CompoundStmt::Create(getContext(), {BodyStmt}, FPOptionsOverride(),
+ SourceLocation(), SourceLocation());
+  }
+  auto *TryStmt = CXXTryStmt::Create(getContext(), Loc, Body, );
 
   EnterCXXTryStmt(*TryStmt);
   emitBodyAndFallthrough(*this, S, TryStmt->getTryBlock());
Index: clang/lib/AST/StmtCXX.cpp
===
--- clang/lib/AST/StmtCXX.cpp
+++ clang/lib/AST/StmtCXX.cpp
@@ -23,7 +23,8 @@
 }
 
 CXXTryStmt *CXXTryStmt::Create(const ASTContext , SourceLocation tryLoc,
-   Stmt *tryBlock, ArrayRef handlers) {
+   CompoundStmt *tryBlock,
+   ArrayRef handlers) {
   const size_t Size = totalSizeToAlloc(handlers.size() + 1);
   void *Mem = C.Allocate(Size, alignof(CXXTryStmt));
   return new (Mem) CXXTryStmt(tryLoc, tryBlock, handlers);
@@ -36,7 +37,7 @@
   return new (Mem) CXXTryStmt(Empty, numHandlers);
 }
 
-CXXTryStmt::CXXTryStmt(SourceLocation tryLoc, Stmt *tryBlock,
+CXXTryStmt::CXXTryStmt(SourceLocation tryLoc, CompoundStmt *tryBlock,
ArrayRef handlers)
 : Stmt(CXXTryStmtClass), TryLoc(tryLoc), NumHandlers(handlers.size()) {
   Stmt **Stmts = getStmts();
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -6793,8 +6793,8 @@
   return ToHandlerOrErr.takeError();
   }
 
-  return CXXTryStmt::Create(
-  Importer.getToContext(), *ToTryLocOrErr,*ToTryBlockOrErr, ToHandlers);
+  return CXXTryStmt::Create(Importer.getToContext(), *ToTryLocOrErr,
+cast(*ToTryBlockOrErr), ToHandlers);
 }
 
 ExpectedStmt ASTNodeImporter::VisitCXXForRangeStmt(CXXForRangeStmt *S) {
Index: clang/include/clang/AST/StmtCXX.h
===
--- clang/include/clang/AST/StmtCXX.h
+++ clang/include/clang/AST/StmtCXX.h
@@ -75,7 +75,8 @@
   unsigned NumHandlers;
   size_t numTrailingObjects(OverloadToken) const { return NumHandlers; }
 
-  CXXTryStmt(SourceLocation tryLoc, Stmt *tryBlock, ArrayRef handlers);
+  CXXTryStmt(SourceLocation tryLoc, CompoundStmt *tryBlock,
+ ArrayRef handlers);
   CXXTryStmt(EmptyShell Empty, unsigned 

[PATCH] D146758: Fix codegen for coroutine with function-try-block

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added inline comments.



Comment at: clang/test/CodeGenCoroutines/coro-function-try-block.cpp:1
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \
+// RUN:-disable-llvm-passes %s -o - | FileCheck %s

bruno wrote:
> Space after `-triple=x86_64`
it is `-triple=x86_64--` (lazy mans way of writing 
`-triple=x86_64-unknown-unknown`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146758

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

[PATCH] D146758: Fix codegen for coroutine with function-try-block

[Matthias Braun via Phabricator via cfe-commits]

MatzeB created this revision.
MatzeB added reviewers: GorNishanov, EricWF, ChuanqiXu, bruno.
Herald added subscribers: modimo, wenlei, martong, mcrosier.
Herald added a reviewer: shafik.
Herald added a project: All.
MatzeB requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This fixes an assertion error when writing a coroutine with a 
function-try-block. In this case the function body is not a `CompoundStmt` so 
the code constructing an artificial CXXTryStmt must also construct a 
`CompoundStmt` for it.

While on it adjust the `CXXStmt::Create` function to only accept 
`CompoundStmt*`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D146758

Files:
  clang/include/clang/AST/StmtCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/StmtCXX.cpp
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CodeGenCoroutines/coro-function-try-block.cpp

Index: clang/test/CodeGenCoroutines/coro-function-try-block.cpp
===
--- /dev/null
+++ clang/test/CodeGenCoroutines/coro-function-try-block.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \
+// RUN:-disable-llvm-passes %s -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+struct task {
+  struct promise_type {
+task get_return_object();
+std::suspend_never initial_suspend();
+std::suspend_never final_suspend() noexcept;
+void return_void();
+void unhandled_exception() noexcept;
+  };
+};
+
+task f() try {
+  co_return;
+} catch(...) {
+}
+
+// CHECK-LABEL: define{{.*}} void @_Z1fv(
+// CHECK: call void @_ZNSt13suspend_never13await_suspendESt16coroutine_handleIvE(
+// CHECK: call void @_ZN4task12promise_type11return_voidEv(
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -4546,7 +4546,8 @@
 
   FSI->setHasCXXTry(TryLoc);
 
-  return CXXTryStmt::Create(Context, TryLoc, TryBlock, Handlers);
+  return CXXTryStmt::Create(Context, TryLoc, cast(TryBlock),
+Handlers);
 }
 
 StmtResult Sema::ActOnSEHTryBlock(bool IsCXXTry, SourceLocation TryLoc,
Index: clang/lib/CodeGen/CGCoroutine.cpp
===
--- clang/lib/CodeGen/CGCoroutine.cpp
+++ clang/lib/CodeGen/CGCoroutine.cpp
@@ -721,8 +721,14 @@
   auto Loc = S.getBeginLoc();
   CXXCatchStmt Catch(Loc, /*exDecl=*/nullptr,
  CurCoro.Data->ExceptionHandler);
-  auto *TryStmt =
-  CXXTryStmt::Create(getContext(), Loc, S.getBody(), );
+  Stmt *BodyStmt = S.getBody();
+  CompoundStmt *Body = dyn_cast(BodyStmt);
+  if (Body == nullptr) {
+Body =
+CompoundStmt::Create(getContext(), {BodyStmt}, FPOptionsOverride(),
+ SourceLocation(), SourceLocation());
+  }
+  auto *TryStmt = CXXTryStmt::Create(getContext(), Loc, Body, );
 
   EnterCXXTryStmt(*TryStmt);
   emitBodyAndFallthrough(*this, S, TryStmt->getTryBlock());
Index: clang/lib/AST/StmtCXX.cpp
===
--- clang/lib/AST/StmtCXX.cpp
+++ clang/lib/AST/StmtCXX.cpp
@@ -23,7 +23,8 @@
 }
 
 CXXTryStmt *CXXTryStmt::Create(const ASTContext , SourceLocation tryLoc,
-   Stmt *tryBlock, ArrayRef handlers) {
+   CompoundStmt *tryBlock,
+   ArrayRef handlers) {
   const size_t Size = totalSizeToAlloc(handlers.size() + 1);
   void *Mem = C.Allocate(Size, alignof(CXXTryStmt));
   return new (Mem) CXXTryStmt(tryLoc, tryBlock, handlers);
@@ -36,7 +37,7 @@
   return new (Mem) CXXTryStmt(Empty, numHandlers);
 }
 
-CXXTryStmt::CXXTryStmt(SourceLocation tryLoc, Stmt *tryBlock,
+CXXTryStmt::CXXTryStmt(SourceLocation tryLoc, CompoundStmt *tryBlock,
ArrayRef handlers)
 : Stmt(CXXTryStmtClass), TryLoc(tryLoc), NumHandlers(handlers.size()) {
   Stmt **Stmts = getStmts();
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -6777,8 +6777,8 @@
   return ToHandlerOrErr.takeError();
   }
 
-  return CXXTryStmt::Create(
-  Importer.getToContext(), *ToTryLocOrErr,*ToTryBlockOrErr, ToHandlers);
+  return CXXTryStmt::Create(Importer.getToContext(), *ToTryLocOrErr,
+cast(*ToTryBlockOrErr), ToHandlers);
 }
 
 ExpectedStmt ASTNodeImporter::VisitCXXForRangeStmt(CXXForRangeStmt *S) {
Index: clang/include/clang/AST/StmtCXX.h
===
--- clang/include/clang/AST/StmtCXX.h
+++ clang/include/clang/AST/StmtCXX.h
@@ -75,7 +75,8 @@
   unsigned NumHandlers;
   size_t 

[PATCH] D144599: [clangd/index/remote]NFC: Adapt code to newer grpc/protobuf versions

[Matthias Braun 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 rGc21378f90a44: [clangd/index/remote]NFC: Adapt code to newer 
grpc/protobuf versions (authored by MatzeB).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144599

Files:
  clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp


Index: clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
===
--- clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
+++ clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
@@ -67,8 +67,9 @@
   google::protobuf::util::MessageToJsonString(Response, , Options);
   if (!JsonStatus.ok()) {
 clang::clangd::elog("Can not convert response ({0}) to JSON ({1}): {2}\n",
-Response.DebugString(), JsonStatus.error_code(),
-JsonStatus.error_message().as_string());
+Response.DebugString(),
+static_cast(JsonStatus.code()),
+JsonStatus.message().as_string());
 return -1;
   }
   llvm::outs() << Output;


Index: clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
===
--- clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
+++ clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
@@ -67,8 +67,9 @@
   google::protobuf::util::MessageToJsonString(Response, , Options);
   if (!JsonStatus.ok()) {
 clang::clangd::elog("Can not convert response ({0}) to JSON ({1}): {2}\n",
-Response.DebugString(), JsonStatus.error_code(),
-JsonStatus.error_message().as_string());
+Response.DebugString(),
+static_cast(JsonStatus.code()),
+JsonStatus.message().as_string());
 return -1;
   }
   llvm::outs() << Output;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144599: [clangd/index/remote]NFC: Adapt code to newer grpc/protobuf versions

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 499909.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144599

Files:
  clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp


Index: clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
===
--- clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
+++ clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
@@ -67,8 +67,9 @@
   google::protobuf::util::MessageToJsonString(Response, , Options);
   if (!JsonStatus.ok()) {
 clang::clangd::elog("Can not convert response ({0}) to JSON ({1}): {2}\n",
-Response.DebugString(), JsonStatus.error_code(),
-JsonStatus.error_message().as_string());
+Response.DebugString(),
+static_cast(JsonStatus.code()),
+JsonStatus.message().as_string());
 return -1;
   }
   llvm::outs() << Output;


Index: clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
===
--- clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
+++ clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
@@ -67,8 +67,9 @@
   google::protobuf::util::MessageToJsonString(Response, , Options);
   if (!JsonStatus.ok()) {
 clang::clangd::elog("Can not convert response ({0}) to JSON ({1}): {2}\n",
-Response.DebugString(), JsonStatus.error_code(),
-JsonStatus.error_message().as_string());
+Response.DebugString(),
+static_cast(JsonStatus.code()),
+JsonStatus.message().as_string());
 return -1;
   }
   llvm::outs() << Output;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144599: [clangd/index/remote]NFC: Adapt code to newer grpc/protobuf versions

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

> Asking as it'd be great to know that we've adoption here, outside of 
> ourselves.

I'm not involved in any of this myself. But @kuganv is :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144599

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

[PATCH] D144599: [clangd/index/remote]NFC: Adapt code to newer grpc/protobuf versions

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

I need this change to fix compilation in our environment which uses 
`grpc-1.42.0`...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144599

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

[PATCH] D144599: [clangd/index/remote]NFC: Adapt code to newer grpc/protobuf versions

[Matthias Braun via Phabricator via cfe-commits]

MatzeB created this revision.
MatzeB added reviewers: kbobyrev, kuganv.
Herald added subscribers: modimo, wenlei, kadircet, arphaman, mcrosier.
Herald added a project: All.
MatzeB requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

It seems newer grpc / protobuf versions renamed
`Status::error_message()` and `Status::error_code()` to `message()`
and `code()` to prepare for replacement with `absl::Status` with the
same names.

As far as I can tell the new names are already available in the
grpc-1.36 version mentioned in the `README` file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144599

Files:
  clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp


Index: clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
===
--- clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
+++ clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
@@ -67,8 +67,8 @@
   google::protobuf::util::MessageToJsonString(Response, , Options);
   if (!JsonStatus.ok()) {
 clang::clangd::elog("Can not convert response ({0}) to JSON ({1}): {2}\n",
-Response.DebugString(), JsonStatus.error_code(),
-JsonStatus.error_message().as_string());
+Response.DebugString(), 
static_cast(JsonStatus.code()),
+JsonStatus.message().as_string());
 return -1;
   }
   llvm::outs() << Output;


Index: clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
===
--- clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
+++ clang-tools-extra/clangd/index/remote/monitor/Monitor.cpp
@@ -67,8 +67,8 @@
   google::protobuf::util::MessageToJsonString(Response, , Options);
   if (!JsonStatus.ok()) {
 clang::clangd::elog("Can not convert response ({0}) to JSON ({1}): {2}\n",
-Response.DebugString(), JsonStatus.error_code(),
-JsonStatus.error_message().as_string());
+Response.DebugString(), static_cast(JsonStatus.code()),
+JsonStatus.message().as_string());
 return -1;
   }
   llvm::outs() << Output;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137475: Explicitly initialize opaque pointer mode in CodeGenAction

[Matthias Braun via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcafe50daf525: Explicitly initialize opaque pointer mode in 
CodeGenAction (authored by MatzeB).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137475

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/CodeGen/Inputs/thinlto-opaque.ll
  clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
  clang/test/CodeGen/thinlto-opaque-typed-mix.ll


Index: clang/test/CodeGen/thinlto-opaque-typed-mix.ll
===
--- /dev/null
+++ clang/test/CodeGen/thinlto-opaque-typed-mix.ll
@@ -0,0 +1,23 @@
+; REQUIRES: x86-registered-target
+; Test that mixing bitcode file with opaque and typed pointers works.
+
+; RUN: mkdir -p %t
+; RUN: opt -module-summary -o %t/typed.bc %s
+; RUN: opt -module-summary -o %t/opaque.bc %S/Inputs/thinlto-opaque.ll
+; RUN: llvm-lto2 run -thinlto-distributed-indexes %t/typed.bc %t/opaque.bc \
+; RUN:   -o %t/native.o -r %t/typed.bc,main,plx -r %t/typed.bc,f2, \
+; RUN:   -r %t/opaque.bc,f2,p
+
+; RUN: %clang_cc1 -triple x86_64-- -emit-obj -o %t/native.o %t/typed.bc \
+; RUN:   -Wno-override-module \
+; RUN:   -fthinlto-index=%t/typed.bc.thinlto.bc
+
+target datalayout = 
"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--"
+
+declare i8* @f2()
+
+define i32 @main() {
+  call i8* @f2()
+  ret i32 0
+}
Index: clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
===
--- clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
+++ clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
@@ -100,7 +100,7 @@
   ; CHECK-IR: br i1 {{.*}}, label %trap
 
   ; We still have to call it as virtual.
-  ; CHECK-IR: %call3 = tail call i32 %7
+  ; CHECK-IR: %call3 = tail call i32 {{%[0-9]+}}
   %call3 = tail call i32 %8(%struct.A* nonnull %obj, i32 %call)
   ret i32 %call3
 }
Index: clang/test/CodeGen/Inputs/thinlto-opaque.ll
===
--- /dev/null
+++ clang/test/CodeGen/Inputs/thinlto-opaque.ll
@@ -0,0 +1,6 @@
+target datalayout = 
"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--"
+
+define ptr @f2() {
+  ret ptr null
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1102,6 +1102,8 @@
   CompilerInstance  = getCompilerInstance();
   SourceManager  = CI.getSourceManager();
 
+  VMContext->setOpaquePointers(CI.getCodeGenOpts().OpaquePointers);
+
   // For ThinLTO backend invocations, ensure that the context
   // merges types based on ODR identifiers. We also need to read
   // the correct module out of a multi-module bitcode file.


Index: clang/test/CodeGen/thinlto-opaque-typed-mix.ll
===
--- /dev/null
+++ clang/test/CodeGen/thinlto-opaque-typed-mix.ll
@@ -0,0 +1,23 @@
+; REQUIRES: x86-registered-target
+; Test that mixing bitcode file with opaque and typed pointers works.
+
+; RUN: mkdir -p %t
+; RUN: opt -module-summary -o %t/typed.bc %s
+; RUN: opt -module-summary -o %t/opaque.bc %S/Inputs/thinlto-opaque.ll
+; RUN: llvm-lto2 run -thinlto-distributed-indexes %t/typed.bc %t/opaque.bc \
+; RUN:   -o %t/native.o -r %t/typed.bc,main,plx -r %t/typed.bc,f2, \
+; RUN:   -r %t/opaque.bc,f2,p
+
+; RUN: %clang_cc1 -triple x86_64-- -emit-obj -o %t/native.o %t/typed.bc \
+; RUN:   -Wno-override-module \
+; RUN:   -fthinlto-index=%t/typed.bc.thinlto.bc
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--"
+
+declare i8* @f2()
+
+define i32 @main() {
+  call i8* @f2()
+  ret i32 0
+}
Index: clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
===
--- clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
+++ clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
@@ -100,7 +100,7 @@
   ; CHECK-IR: br i1 {{.*}}, label %trap
 
   ; We still have to call it as virtual.
-  ; CHECK-IR: %call3 = tail call i32 %7
+  ; CHECK-IR: %call3 = tail call i32 {{%[0-9]+}}
   %call3 = tail call i32 %8(%struct.A* nonnull %obj, i32 %call)
   ret i32 %call3
 }
Index: clang/test/CodeGen/Inputs/thinlto-opaque.ll
===
--- /dev/null
+++ clang/test/CodeGen/Inputs/thinlto-opaque.ll
@@ -0,0 +1,6 @@
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--"
+
+define ptr @f2() {
+  ret ptr null
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp

[PATCH] D137475: Explicitly initialize opaque pointer mode in CodeGenAction

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 473747.
MatzeB retitled this revision from "Explicitly initialize opaque pointer mode 
when -fthinlto-index is used" to "Explicitly initialize opaque pointer mode in 
CodeGenAction".
MatzeB edited the summary of this revision.
MatzeB added a comment.

address review feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137475

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/CodeGen/Inputs/thinlto-opaque.ll
  clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
  clang/test/CodeGen/thinlto-opaque-typed-mix.ll


Index: clang/test/CodeGen/thinlto-opaque-typed-mix.ll
===
--- /dev/null
+++ clang/test/CodeGen/thinlto-opaque-typed-mix.ll
@@ -0,0 +1,23 @@
+; REQUIRES: x86-registered-target
+; Test that mixing bitcode file with opaque and typed pointers works.
+
+; RUN: mkdir -p %t
+; RUN: opt -module-summary -o %t/typed.bc %s
+; RUN: opt -module-summary -o %t/opaque.bc %S/Inputs/thinlto-opaque.ll
+; RUN: llvm-lto2 run -thinlto-distributed-indexes %t/typed.bc %t/opaque.bc \
+; RUN:   -o %t/native.o -r %t/typed.bc,main,plx -r %t/typed.bc,f2, \
+; RUN:   -r %t/opaque.bc,f2,p
+
+; RUN: %clang_cc1 -triple x86_64-- -emit-obj -o %t/native.o %t/typed.bc \
+; RUN:   -Wno-override-module \
+; RUN:   -fthinlto-index=%t/typed.bc.thinlto.bc
+
+target datalayout = 
"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--"
+
+declare i8* @f2()
+
+define i32 @main() {
+  call i8* @f2()
+  ret i32 0
+}
Index: clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
===
--- clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
+++ clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
@@ -100,7 +100,7 @@
   ; CHECK-IR: br i1 {{.*}}, label %trap
 
   ; We still have to call it as virtual.
-  ; CHECK-IR: %call3 = tail call i32 %7
+  ; CHECK-IR: %call3 = tail call i32 {{%[0-9]+}}
   %call3 = tail call i32 %8(%struct.A* nonnull %obj, i32 %call)
   ret i32 %call3
 }
Index: clang/test/CodeGen/Inputs/thinlto-opaque.ll
===
--- /dev/null
+++ clang/test/CodeGen/Inputs/thinlto-opaque.ll
@@ -0,0 +1,6 @@
+target datalayout = 
"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--"
+
+define ptr @f2() {
+  ret ptr null
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1102,6 +1102,8 @@
   CompilerInstance  = getCompilerInstance();
   SourceManager  = CI.getSourceManager();
 
+  VMContext->setOpaquePointers(CI.getCodeGenOpts().OpaquePointers);
+
   // For ThinLTO backend invocations, ensure that the context
   // merges types based on ODR identifiers. We also need to read
   // the correct module out of a multi-module bitcode file.


Index: clang/test/CodeGen/thinlto-opaque-typed-mix.ll
===
--- /dev/null
+++ clang/test/CodeGen/thinlto-opaque-typed-mix.ll
@@ -0,0 +1,23 @@
+; REQUIRES: x86-registered-target
+; Test that mixing bitcode file with opaque and typed pointers works.
+
+; RUN: mkdir -p %t
+; RUN: opt -module-summary -o %t/typed.bc %s
+; RUN: opt -module-summary -o %t/opaque.bc %S/Inputs/thinlto-opaque.ll
+; RUN: llvm-lto2 run -thinlto-distributed-indexes %t/typed.bc %t/opaque.bc \
+; RUN:   -o %t/native.o -r %t/typed.bc,main,plx -r %t/typed.bc,f2, \
+; RUN:   -r %t/opaque.bc,f2,p
+
+; RUN: %clang_cc1 -triple x86_64-- -emit-obj -o %t/native.o %t/typed.bc \
+; RUN:   -Wno-override-module \
+; RUN:   -fthinlto-index=%t/typed.bc.thinlto.bc
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--"
+
+declare i8* @f2()
+
+define i32 @main() {
+  call i8* @f2()
+  ret i32 0
+}
Index: clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
===
--- clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
+++ clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
@@ -100,7 +100,7 @@
   ; CHECK-IR: br i1 {{.*}}, label %trap
 
   ; We still have to call it as virtual.
-  ; CHECK-IR: %call3 = tail call i32 %7
+  ; CHECK-IR: %call3 = tail call i32 {{%[0-9]+}}
   %call3 = tail call i32 %8(%struct.A* nonnull %obj, i32 %call)
   ret i32 %call3
 }
Index: clang/test/CodeGen/Inputs/thinlto-opaque.ll
===
--- /dev/null
+++ clang/test/CodeGen/Inputs/thinlto-opaque.ll
@@ -0,0 +1,6 @@
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--"
+
+define ptr @f2() {
+  ret ptr null
+}
Index: 

[PATCH] D137475: Explicitely initialize opaque pointer mode when -fthinlto-index is used

[Matthias Braun via Phabricator via cfe-commits]

MatzeB created this revision.
MatzeB added reviewers: weiwang, aeubanks, nikic, MaskRay.
Herald added subscribers: ormris, StephenFan, modimo, wenlei, arphaman, 
steven_wu, hiraditya, inglorion, mcrosier.
Herald added a project: All.
MatzeB requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Explicitly call `LLVMContext::setOpaquePointers` before loading any bitcode 
files when `-fthinlto-index` is used. Without this we can run into problematic 
situations when bitcode files with typed and opaque pointers are mixed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137475

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/CodeGen/Inputs/thinlto-opaque.ll
  clang/test/CodeGen/thinlto-opaque-typed-mix.ll


Index: clang/test/CodeGen/thinlto-opaque-typed-mix.ll
===
--- /dev/null
+++ clang/test/CodeGen/thinlto-opaque-typed-mix.ll
@@ -0,0 +1,24 @@
+; REQUIRES: x86-registered-target
+; Test that mixing bitcode file with opaque and typed pointers works.
+
+; RUN: mkdir -p %t
+; RUN: opt -module-summary -o %t/typed.bc %s
+; RUN: opt -module-summary -o %t/opaque.bc %S/Inputs/thinlto-opaque.ll
+; RUN: llvm-lto2 run -thinlto-distributed-indexes %t/typed.bc %t/opaque.bc \
+; RUN:   -o %t/native.o -r %t/typed.bc,main,plx -r %t/typed.bc,f2, \
+; RUN:   -r %t/opaque.bc,f2,p
+
+; RUN: %clang_cc1 -triple x86_64-- -emit-obj -o %t/native.o %t/typed.bc \
+; RUN:   -Wno-override-module \
+; RUN:   -fthinlto-index=%t/typed.bc.thinlto.bc
+; RUN: false
+
+target datalayout = 
"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--"
+
+declare i8* @f2()
+
+define i32 @main() {
+  call i8* @f2()
+  ret i32 0
+}
Index: clang/test/CodeGen/Inputs/thinlto-opaque.ll
===
--- /dev/null
+++ clang/test/CodeGen/Inputs/thinlto-opaque.ll
@@ -0,0 +1,6 @@
+target datalayout = 
"e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--"
+
+define ptr @f2() {
+  ret ptr null
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1106,6 +1106,11 @@
   M->setTargetTriple(CI.getTargetOpts().Triple);
   return M;
 }
+
+// For ThinLTO explicitely set opaque pointer mode to avoid problems when
+// mixing opaque pointers and typed pointers bitcode files.
+VMContext->setOpaquePointers(CI.getCodeGenOpts().OpaquePointers);
+
 Expected> MOrErr =
 Bm->parseModule(*VMContext);
 if (!MOrErr)


Index: clang/test/CodeGen/thinlto-opaque-typed-mix.ll
===
--- /dev/null
+++ clang/test/CodeGen/thinlto-opaque-typed-mix.ll
@@ -0,0 +1,24 @@
+; REQUIRES: x86-registered-target
+; Test that mixing bitcode file with opaque and typed pointers works.
+
+; RUN: mkdir -p %t
+; RUN: opt -module-summary -o %t/typed.bc %s
+; RUN: opt -module-summary -o %t/opaque.bc %S/Inputs/thinlto-opaque.ll
+; RUN: llvm-lto2 run -thinlto-distributed-indexes %t/typed.bc %t/opaque.bc \
+; RUN:   -o %t/native.o -r %t/typed.bc,main,plx -r %t/typed.bc,f2, \
+; RUN:   -r %t/opaque.bc,f2,p
+
+; RUN: %clang_cc1 -triple x86_64-- -emit-obj -o %t/native.o %t/typed.bc \
+; RUN:   -Wno-override-module \
+; RUN:   -fthinlto-index=%t/typed.bc.thinlto.bc
+; RUN: false
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--"
+
+declare i8* @f2()
+
+define i32 @main() {
+  call i8* @f2()
+  ret i32 0
+}
Index: clang/test/CodeGen/Inputs/thinlto-opaque.ll
===
--- /dev/null
+++ clang/test/CodeGen/Inputs/thinlto-opaque.ll
@@ -0,0 +1,6 @@
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--"
+
+define ptr @f2() {
+  ret ptr null
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1106,6 +1106,11 @@
   M->setTargetTriple(CI.getTargetOpts().Triple);
   return M;
 }
+
+// For ThinLTO explicitely set opaque pointer mode to avoid problems when
+// mixing opaque pointers and typed pointers bitcode files.
+VMContext->setOpaquePointers(CI.getCodeGenOpts().OpaquePointers);
+
 Expected> MOrErr =
 Bm->parseModule(*VMContext);
 if (!MOrErr)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125847: LTO: Add option to initialize with opaque/non-opaque pointer types

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

In D125847#3552297 , @MaskRay wrote:

> Your commit message seems to use the original summary.

Yes sorry, I noticed it too after pushing it. I think this can‘t be fixed after 
the push though…


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

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

[PATCH] D125847: LTO: Add option to initialize with opaque/non-opaque pointer types

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

@JakeEgan I did a quick look for the code in that stacktrace but nothing jumped 
out on me. It‘s with inlining effects obscuring it. But I think that crash must 
be investigated on an AIX machine, there‘s no indication that this directly 
related to my change which works on all other systems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

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

[PATCH] D125847: LTO: Add option to initialize with opaque/non-opaque pointer types

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added inline comments.



Comment at: lld/test/ELF/lto/discard-value-names.ll:4
 
 ; RUN: ld.lld -shared -save-temps %t.o -o %t2.o
 ; RUN: llvm-dis < %t2.o.0.0.preopt.bc | FileCheck %s

bmahjour wrote:
> I see `--plugin-opt=opaque-pointers` is being explicitly specified for some 
> but not all tests. I suppose the reason you explicitly specify it is to make 
> sure the tests pass even for builds where opaque pointers are disabled. If 
> that's the case, why aren't you doing it consistently?
It‘s not specified in any of the lld tests except the two tests for the option 
itself (because in lld lto its enabled by default now).

It is specified for clang (but not clang cc1) in a coupletests because the 
clang driver drfaults can currently be changed with a cmake flag.




Comment at: llvm/test/Analysis/StackSafetyAnalysis/ipa.ll:86
 
-; RUN: llvm-lto2 run %t.summ0.bc %t.summ1.bc -o %t.lto -stack-safety-print 
-stack-safety-run -save-temps -thinlto-threads 1 -O0 \
+; RUN: llvm-lto2 run -opaque-pointers=0 %t.summ0.bc %t.summ1.bc -o %t.lto 
-stack-safety-print -stack-safety-run -save-temps -thinlto-threads 1 -O0 \
 ; RUN:  $(cat %t.res.txt) \

bmahjour wrote:
> why does this test (and ipa-alias.ll above) need to run with opaque pointers 
> off?
I don‘t remember this test specifically but some were tricky to update, feel 
free to submit a patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

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

[PATCH] D125847: LTO: Add option to initialize with opaque/non-opaque pointer types

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.



In D125847#3554569 , @Jake-Egan wrote:

> Hi, this caused `arm-float-abi-lto.c` to fail on AIX. The failure went away 
> for a few builds, then came back. Could you take a look?
>
> https://lab.llvm.org/buildbot/#/builders/214/builds/1625/steps/6/logs/FAIL__Clang__arm-float-abi-lto_c

Uh, this change just enables opaque pointers for this test. And it appears to 
be crashing nondeterministically somewhere within LLVM. Unfortunately I don't 
see this happening on my X86 development machine. None of the ASAN enabled 
buildbots appears to complain either. I don't know how to even start debugging 
this, do you have at least a stacktrace?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

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

[PATCH] D125847: LTO: Add option to initialize with opaque/non-opaque pointer types

[Matthias Braun 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 rG850d53a197f9: LTO: Decide upfront whether to use 
opaque/non-opaque pointer types (authored by MatzeB).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/CodeGen/thinlto-inline-asm2.c
  clang/test/Driver/arm-float-abi-lto.c
  clang/test/Driver/lto-no-opaque-pointers.c
  clang/test/Driver/lto-opaque-pointers.c
  clang/test/Driver/memtag_lto.c
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/LTO.cpp
  lld/ELF/Options.td
  lld/test/ELF/lto/discard-value-names.ll
  lld/test/ELF/lto/ltopasses-basic.ll
  lld/test/ELF/lto/type-merge.ll
  lld/test/ELF/lto/type-merge2.ll
  lld/test/ELF/lto/wrap-unreferenced-before-codegen.test
  llvm/docs/OpaquePointers.rst
  llvm/include/llvm/LTO/Config.h
  llvm/test/Analysis/StackSafetyAnalysis/ipa-alias.ll
  llvm/test/Analysis/StackSafetyAnalysis/ipa.ll
  llvm/test/LTO/Resolution/X86/alias-alias.ll
  llvm/test/LTO/Resolution/X86/comdat.ll
  llvm/test/LTO/Resolution/X86/ifunc2.ll
  llvm/test/LTO/Resolution/X86/local-def-dllimport.ll
  llvm/test/LTO/X86/Inputs/opaque-pointers.ll
  llvm/test/LTO/X86/cfi_jt_aliases.ll
  llvm/test/LTO/X86/mix-opaque-typed.ll
  llvm/test/LTO/X86/type-mapping-bug4.ll
  llvm/test/ThinLTO/X86/Inputs/import-constant.ll
  llvm/test/ThinLTO/X86/cfi-devirt.ll
  llvm/test/ThinLTO/X86/cfi-unsat.ll
  llvm/test/ThinLTO/X86/devirt-after-icp.ll
  llvm/test/ThinLTO/X86/devirt2.ll
  llvm/test/ThinLTO/X86/devirt_check.ll
  llvm/test/ThinLTO/X86/devirt_promote.ll
  llvm/test/ThinLTO/X86/devirt_single_hybrid.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-unknown.ll
  llvm/test/ThinLTO/X86/globals-import-blockaddr.ll
  llvm/test/ThinLTO/X86/import-constant.ll
  llvm/test/ThinLTO/X86/import-dsolocal.ll
  llvm/test/ThinLTO/X86/index-const-prop-gvref-pie.ll
  llvm/test/ThinLTO/X86/index-const-prop-gvref.ll
  llvm/test/ThinLTO/X86/index-const-prop-linkage.ll
  llvm/test/ThinLTO/X86/reference_non_importable.ll
  llvm/test/ThinLTO/X86/weak_externals.ll
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-lto2/llvm-lto2.cpp

Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -143,6 +143,10 @@
  cl::desc("Run PGO context sensitive IR instrumentation"),
  cl::init(false), cl::Hidden);
 
+static cl::opt LtoOpaquePointers("lto-opaque-pointers",
+   cl::desc("Enable opaque pointer types"),
+   cl::init(true), cl::Hidden);
+
 static cl::opt
 DebugPassManager("debug-pass-manager", cl::init(false), cl::Hidden,
  cl::desc("Print pass management debugging information"));
@@ -291,6 +295,7 @@
   Conf.StatsFile = StatsFile;
   Conf.PTO.LoopVectorization = Conf.OptLevel > 1;
   Conf.PTO.SLPVectorization = Conf.OptLevel > 1;
+  Conf.OpaquePointers = LtoOpaquePointers;
 
   ThinBackend Backend;
   if (ThinLTODistributedIndexes)
Index: llvm/tools/gold/gold-plugin.cpp
===
--- llvm/tools/gold/gold-plugin.cpp
+++ llvm/tools/gold/gold-plugin.cpp
@@ -208,6 +208,8 @@
   static std::string stats_file;
   // Asserts that LTO link has whole program visibility
   static bool whole_program_visibility = false;
+  // Use opaque pointer types.
+  static bool opaque_pointers = true;
 
   // Optimization remarks filename, accepted passes and hotness options
   static std::string RemarksFilename;
@@ -308,6 +310,10 @@
   RemarksFormat = std::string(opt);
 } else if (opt.consume_front("stats-file=")) {
   stats_file = std::string(opt);
+} else if (opt == "opaque-pointers") {
+  opaque_pointers = true;
+} else if (opt == "no-opaque-pointers") {
+  opaque_pointers = false;
 } else {
   // Save this option to pass to the code generator.
   // ParseCommandLineOptions() expects argv[0] to be program name. Lazily
@@ -957,6 +963,8 @@
 
   Conf.HasWholeProgramVisibility = options::whole_program_visibility;
 
+  Conf.OpaquePointers = options.opaque_pointers;
+
   Conf.StatsFile = options::stats_file;
   return std::make_unique(std::move(Conf), Backend,
 options::ParallelCodeGenParallelismLevel);
Index: llvm/test/ThinLTO/X86/weak_externals.ll
===
--- llvm/test/ThinLTO/X86/weak_externals.ll
+++ llvm/test/ThinLTO/X86/weak_externals.ll
@@ -10,9 +10,9 @@
 ; RUN: llvm-dis %t.out.1.2.internalize.bc -o - | FileCheck %s --check-prefix=INTERNALIZE
 
 ; CHECK: @_ZZN9SingletonI1SE11getInstanceEvE8instance = available_externally dso_local global %struct.S zeroinitializer
-; 

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

In D125847#3551972 , @MaskRay wrote:

> In D125847#3551841 , @MatzeB wrote:
>
>> address review feedback. I assume this is accepted and good to push when the 
>> buildkit builds look reasonable.
>
> (I changed myself to a blocking review to check the component I mostly care 
> about. I promise I'll perform a quick check.)

Of course, I won't push then. I wasn't sure since the accept came after the set 
to block.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

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

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 433591.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/CodeGen/thinlto-inline-asm2.c
  clang/test/Driver/arm-float-abi-lto.c
  clang/test/Driver/lto-no-opaque-pointers.c
  clang/test/Driver/lto-opaque-pointers.c
  clang/test/Driver/memtag_lto.c
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/LTO.cpp
  lld/ELF/Options.td
  lld/test/ELF/lto/discard-value-names.ll
  lld/test/ELF/lto/ltopasses-basic.ll
  lld/test/ELF/lto/type-merge.ll
  lld/test/ELF/lto/type-merge2.ll
  lld/test/ELF/lto/wrap-unreferenced-before-codegen.test
  llvm/docs/OpaquePointers.rst
  llvm/include/llvm/LTO/Config.h
  llvm/test/Analysis/StackSafetyAnalysis/ipa-alias.ll
  llvm/test/Analysis/StackSafetyAnalysis/ipa.ll
  llvm/test/LTO/Resolution/X86/alias-alias.ll
  llvm/test/LTO/Resolution/X86/comdat.ll
  llvm/test/LTO/Resolution/X86/ifunc2.ll
  llvm/test/LTO/Resolution/X86/local-def-dllimport.ll
  llvm/test/LTO/X86/Inputs/opaque-pointers.ll
  llvm/test/LTO/X86/cfi_jt_aliases.ll
  llvm/test/LTO/X86/mix-opaque-typed.ll
  llvm/test/LTO/X86/type-mapping-bug4.ll
  llvm/test/ThinLTO/X86/Inputs/import-constant.ll
  llvm/test/ThinLTO/X86/cfi-devirt.ll
  llvm/test/ThinLTO/X86/cfi-unsat.ll
  llvm/test/ThinLTO/X86/devirt-after-icp.ll
  llvm/test/ThinLTO/X86/devirt2.ll
  llvm/test/ThinLTO/X86/devirt_check.ll
  llvm/test/ThinLTO/X86/devirt_promote.ll
  llvm/test/ThinLTO/X86/devirt_single_hybrid.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-unknown.ll
  llvm/test/ThinLTO/X86/globals-import-blockaddr.ll
  llvm/test/ThinLTO/X86/import-constant.ll
  llvm/test/ThinLTO/X86/import-dsolocal.ll
  llvm/test/ThinLTO/X86/index-const-prop-gvref-pie.ll
  llvm/test/ThinLTO/X86/index-const-prop-gvref.ll
  llvm/test/ThinLTO/X86/index-const-prop-linkage.ll
  llvm/test/ThinLTO/X86/reference_non_importable.ll
  llvm/test/ThinLTO/X86/weak_externals.ll
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-lto2/llvm-lto2.cpp

Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -143,6 +143,10 @@
  cl::desc("Run PGO context sensitive IR instrumentation"),
  cl::init(false), cl::Hidden);
 
+static cl::opt LtoOpaquePointers("lto-opaque-pointers",
+   cl::desc("Enable opaque pointer types"),
+   cl::init(true), cl::Hidden);
+
 static cl::opt
 DebugPassManager("debug-pass-manager", cl::init(false), cl::Hidden,
  cl::desc("Print pass management debugging information"));
@@ -291,6 +295,7 @@
   Conf.StatsFile = StatsFile;
   Conf.PTO.LoopVectorization = Conf.OptLevel > 1;
   Conf.PTO.SLPVectorization = Conf.OptLevel > 1;
+  Conf.OpaquePointers = LtoOpaquePointers;
 
   ThinBackend Backend;
   if (ThinLTODistributedIndexes)
Index: llvm/tools/gold/gold-plugin.cpp
===
--- llvm/tools/gold/gold-plugin.cpp
+++ llvm/tools/gold/gold-plugin.cpp
@@ -208,6 +208,8 @@
   static std::string stats_file;
   // Asserts that LTO link has whole program visibility
   static bool whole_program_visibility = false;
+  // Use opaque pointer types.
+  static bool opaque_pointers = true;
 
   // Optimization remarks filename, accepted passes and hotness options
   static std::string RemarksFilename;
@@ -308,6 +310,10 @@
   RemarksFormat = std::string(opt);
 } else if (opt.consume_front("stats-file=")) {
   stats_file = std::string(opt);
+} else if (opt == "opaque-pointers") {
+  opaque_pointers = true;
+} else if (opt == "no-opaque-pointers") {
+  opaque_pointers = false;
 } else {
   // Save this option to pass to the code generator.
   // ParseCommandLineOptions() expects argv[0] to be program name. Lazily
@@ -957,6 +963,8 @@
 
   Conf.HasWholeProgramVisibility = options::whole_program_visibility;
 
+  Conf.OpaquePointers = options.opaque_pointers;
+
   Conf.StatsFile = options::stats_file;
   return std::make_unique(std::move(Conf), Backend,
 options::ParallelCodeGenParallelismLevel);
Index: llvm/test/ThinLTO/X86/weak_externals.ll
===
--- llvm/test/ThinLTO/X86/weak_externals.ll
+++ llvm/test/ThinLTO/X86/weak_externals.ll
@@ -10,9 +10,9 @@
 ; RUN: llvm-dis %t.out.1.2.internalize.bc -o - | FileCheck %s --check-prefix=INTERNALIZE
 
 ; CHECK: @_ZZN9SingletonI1SE11getInstanceEvE8instance = available_externally dso_local global %struct.S zeroinitializer
-; CHECK: @_ZZN9SingletonI1SE11getInstanceEvE13instance_weak = available_externally dso_local global %struct.S* null, align 8
-; CHECK: define linkonce_odr dso_local dereferenceable(16) %struct.S* 

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added inline comments.



Comment at: llvm/tools/gold/gold-plugin.cpp:966
 
+  Config.OpaquePointers = options.opaque_pointers;
+

MaskRay wrote:
> `Conf`?
> 
> 
Ouch, good catch. Guess we have no form of direct testing of the gold plugin in 
LLVM...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

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

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 433577.
MatzeB added a comment.

address review feedback. I assume this is accepted and good to push when the 
buildkit builds look reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/CodeGen/thinlto-inline-asm2.c
  clang/test/Driver/arm-float-abi-lto.c
  clang/test/Driver/lto-no-opaque-pointers.c
  clang/test/Driver/lto-opaque-pointers.c
  clang/test/Driver/memtag_lto.c
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/LTO.cpp
  lld/ELF/Options.td
  lld/test/ELF/lto/discard-value-names.ll
  lld/test/ELF/lto/ltopasses-basic.ll
  lld/test/ELF/lto/type-merge.ll
  lld/test/ELF/lto/type-merge2.ll
  lld/test/ELF/lto/wrap-unreferenced-before-codegen.test
  llvm/docs/OpaquePointers.rst
  llvm/include/llvm/LTO/Config.h
  llvm/test/Analysis/StackSafetyAnalysis/ipa-alias.ll
  llvm/test/Analysis/StackSafetyAnalysis/ipa.ll
  llvm/test/LTO/Resolution/X86/alias-alias.ll
  llvm/test/LTO/Resolution/X86/comdat.ll
  llvm/test/LTO/Resolution/X86/ifunc2.ll
  llvm/test/LTO/Resolution/X86/local-def-dllimport.ll
  llvm/test/LTO/X86/Inputs/opaque-pointers.ll
  llvm/test/LTO/X86/cfi_jt_aliases.ll
  llvm/test/LTO/X86/mix-opaque-typed.ll
  llvm/test/LTO/X86/type-mapping-bug4.ll
  llvm/test/ThinLTO/X86/Inputs/import-constant.ll
  llvm/test/ThinLTO/X86/cfi-devirt.ll
  llvm/test/ThinLTO/X86/cfi-unsat.ll
  llvm/test/ThinLTO/X86/devirt-after-icp.ll
  llvm/test/ThinLTO/X86/devirt2.ll
  llvm/test/ThinLTO/X86/devirt_check.ll
  llvm/test/ThinLTO/X86/devirt_promote.ll
  llvm/test/ThinLTO/X86/devirt_single_hybrid.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-unknown.ll
  llvm/test/ThinLTO/X86/globals-import-blockaddr.ll
  llvm/test/ThinLTO/X86/import-constant.ll
  llvm/test/ThinLTO/X86/import-dsolocal.ll
  llvm/test/ThinLTO/X86/index-const-prop-gvref-pie.ll
  llvm/test/ThinLTO/X86/index-const-prop-gvref.ll
  llvm/test/ThinLTO/X86/index-const-prop-linkage.ll
  llvm/test/ThinLTO/X86/reference_non_importable.ll
  llvm/test/ThinLTO/X86/weak_externals.ll
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-lto2/llvm-lto2.cpp

Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -143,6 +143,10 @@
  cl::desc("Run PGO context sensitive IR instrumentation"),
  cl::init(false), cl::Hidden);
 
+static cl::opt LtoOpaquePointers("lto-opaque-pointers",
+   cl::desc("Enable opaque pointer types"),
+   cl::init(true), cl::Hidden);
+
 static cl::opt
 DebugPassManager("debug-pass-manager", cl::init(false), cl::Hidden,
  cl::desc("Print pass management debugging information"));
@@ -291,6 +295,7 @@
   Conf.StatsFile = StatsFile;
   Conf.PTO.LoopVectorization = Conf.OptLevel > 1;
   Conf.PTO.SLPVectorization = Conf.OptLevel > 1;
+  Conf.OpaquePointers = LtoOpaquePointers;
 
   ThinBackend Backend;
   if (ThinLTODistributedIndexes)
Index: llvm/tools/gold/gold-plugin.cpp
===
--- llvm/tools/gold/gold-plugin.cpp
+++ llvm/tools/gold/gold-plugin.cpp
@@ -208,6 +208,8 @@
   static std::string stats_file;
   // Asserts that LTO link has whole program visibility
   static bool whole_program_visibility = false;
+  // Use opaque pointer types.
+  static bool opaque_pointers = true;
 
   // Optimization remarks filename, accepted passes and hotness options
   static std::string RemarksFilename;
@@ -308,6 +310,10 @@
   RemarksFormat = std::string(opt);
 } else if (opt.consume_front("stats-file=")) {
   stats_file = std::string(opt);
+} else if (opt == "opaque-pointers") {
+  opaque_pointers = true;
+} else if (opt == "no-opaque-pointers") {
+  opaque_pointers = false;
 } else {
   // Save this option to pass to the code generator.
   // ParseCommandLineOptions() expects argv[0] to be program name. Lazily
@@ -957,6 +963,8 @@
 
   Conf.HasWholeProgramVisibility = options::whole_program_visibility;
 
+  Config.OpaquePointers = options.opaque_pointers;
+
   Conf.StatsFile = options::stats_file;
   return std::make_unique(std::move(Conf), Backend,
 options::ParallelCodeGenParallelismLevel);
Index: llvm/test/ThinLTO/X86/weak_externals.ll
===
--- llvm/test/ThinLTO/X86/weak_externals.ll
+++ llvm/test/ThinLTO/X86/weak_externals.ll
@@ -10,9 +10,9 @@
 ; RUN: llvm-dis %t.out.1.2.internalize.bc -o - | FileCheck %s --check-prefix=INTERNALIZE
 
 ; CHECK: @_ZZN9SingletonI1SE11getInstanceEvE8instance = available_externally dso_local global %struct.S zeroinitializer
-; CHECK: @_ZZN9SingletonI1SE11getInstanceEvE13instance_weak = 

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 432350.
MatzeB marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/CodeGen/thinlto-inline-asm2.c
  clang/test/Driver/arm-float-abi-lto.c
  clang/test/Driver/lto-no-opaque-pointers.c
  clang/test/Driver/lto-opaque-pointers.c
  clang/test/Driver/memtag_lto.c
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/LTO.cpp
  lld/ELF/Options.td
  lld/test/ELF/lto/discard-value-names.ll
  lld/test/ELF/lto/ltopasses-basic.ll
  lld/test/ELF/lto/type-merge.ll
  lld/test/ELF/lto/type-merge2.ll
  lld/test/ELF/lto/wrap-unreferenced-before-codegen.test
  llvm/docs/OpaquePointers.rst
  llvm/include/llvm/LTO/Config.h
  llvm/test/Analysis/StackSafetyAnalysis/ipa-alias.ll
  llvm/test/Analysis/StackSafetyAnalysis/ipa.ll
  llvm/test/LTO/Resolution/X86/alias-alias.ll
  llvm/test/LTO/Resolution/X86/comdat.ll
  llvm/test/LTO/Resolution/X86/ifunc2.ll
  llvm/test/LTO/Resolution/X86/local-def-dllimport.ll
  llvm/test/LTO/X86/Inputs/opaque-pointers.ll
  llvm/test/LTO/X86/cfi_jt_aliases.ll
  llvm/test/LTO/X86/mix-opaque-typed.ll
  llvm/test/LTO/X86/type-mapping-bug4.ll
  llvm/test/ThinLTO/X86/Inputs/import-constant.ll
  llvm/test/ThinLTO/X86/cfi-devirt.ll
  llvm/test/ThinLTO/X86/cfi-unsat.ll
  llvm/test/ThinLTO/X86/devirt-after-icp.ll
  llvm/test/ThinLTO/X86/devirt2.ll
  llvm/test/ThinLTO/X86/devirt_check.ll
  llvm/test/ThinLTO/X86/devirt_promote.ll
  llvm/test/ThinLTO/X86/devirt_single_hybrid.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-unknown.ll
  llvm/test/ThinLTO/X86/globals-import-blockaddr.ll
  llvm/test/ThinLTO/X86/import-constant.ll
  llvm/test/ThinLTO/X86/import-dsolocal.ll
  llvm/test/ThinLTO/X86/index-const-prop-gvref-pie.ll
  llvm/test/ThinLTO/X86/index-const-prop-gvref.ll
  llvm/test/ThinLTO/X86/index-const-prop-linkage.ll
  llvm/test/ThinLTO/X86/reference_non_importable.ll
  llvm/test/ThinLTO/X86/weak_externals.ll
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-lto2/llvm-lto2.cpp

Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -143,6 +143,10 @@
  cl::desc("Run PGO context sensitive IR instrumentation"),
  cl::init(false), cl::Hidden);
 
+static cl::opt LtoOpaquePointers("lto-opaque-pointers",
+   cl::desc("Enable opaque pointer types"),
+   cl::init(true), cl::Hidden);
+
 static cl::opt
 DebugPassManager("debug-pass-manager", cl::init(false), cl::Hidden,
  cl::desc("Print pass management debugging information"));
@@ -291,6 +295,7 @@
   Conf.StatsFile = StatsFile;
   Conf.PTO.LoopVectorization = Conf.OptLevel > 1;
   Conf.PTO.SLPVectorization = Conf.OptLevel > 1;
+  Conf.OpaquePointers = LtoOpaquePointers;
 
   ThinBackend Backend;
   if (ThinLTODistributedIndexes)
Index: llvm/tools/gold/gold-plugin.cpp
===
--- llvm/tools/gold/gold-plugin.cpp
+++ llvm/tools/gold/gold-plugin.cpp
@@ -208,6 +208,8 @@
   static std::string stats_file;
   // Asserts that LTO link has whole program visibility
   static bool whole_program_visibility = false;
+  // Use opaque pointer types.
+  static bool opaque_pointers = true;
 
   // Optimization remarks filename, accepted passes and hotness options
   static std::string RemarksFilename;
@@ -308,6 +310,10 @@
   RemarksFormat = std::string(opt);
 } else if (opt.consume_front("stats-file=")) {
   stats_file = std::string(opt);
+} else if (opt == "opaque-pointers") {
+  opaque_pointers = true;
+} else if (opt == "no-opaque-pointers") {
+  opaque_pointers = false;
 } else {
   // Save this option to pass to the code generator.
   // ParseCommandLineOptions() expects argv[0] to be program name. Lazily
@@ -957,6 +963,8 @@
 
   Conf.HasWholeProgramVisibility = options::whole_program_visibility;
 
+  Config.OpaquePointers = options.opaque_pointers;
+
   Conf.StatsFile = options::stats_file;
   return std::make_unique(std::move(Conf), Backend,
 options::ParallelCodeGenParallelismLevel);
Index: llvm/test/ThinLTO/X86/weak_externals.ll
===
--- llvm/test/ThinLTO/X86/weak_externals.ll
+++ llvm/test/ThinLTO/X86/weak_externals.ll
@@ -10,9 +10,9 @@
 ; RUN: llvm-dis %t.out.1.2.internalize.bc -o - | FileCheck %s --check-prefix=INTERNALIZE
 
 ; CHECK: @_ZZN9SingletonI1SE11getInstanceEvE8instance = available_externally dso_local global %struct.S zeroinitializer
-; CHECK: @_ZZN9SingletonI1SE11getInstanceEvE13instance_weak = available_externally dso_local global %struct.S* null, align 8
-; CHECK: define linkonce_odr dso_local 

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

[Matthias Braun via Phabricator via cfe-commits]

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



Comment at: lld/ELF/Options.td:311
+  "Use opaque pointers in IR used during LTO",
+  "Use typed pointers in IR used during LTO (default)">;
+

nikic wrote:
> `(default)` here is outdated.
> 
> Possibly this option should be `lto_opaque_pointers`? That seems to be the 
> convention for other related options.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

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

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 432347.
MatzeB marked an inline comment as done.
MatzeB added a comment.

Address review feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/CodeGen/thinlto-inline-asm2.c
  clang/test/Driver/arm-float-abi-lto.c
  clang/test/Driver/lto-no-opaque-pointers.c
  clang/test/Driver/lto-opaque-pointers.c
  clang/test/Driver/memtag_lto.c
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/LTO.cpp
  lld/ELF/Options.td
  lld/test/ELF/lto/discard-value-names.ll
  lld/test/ELF/lto/ltopasses-basic.ll
  lld/test/ELF/lto/type-merge.ll
  lld/test/ELF/lto/type-merge2.ll
  lld/test/ELF/lto/wrap-unreferenced-before-codegen.test
  llvm/docs/OpaquePointers.rst
  llvm/include/llvm/LTO/Config.h
  llvm/test/Analysis/StackSafetyAnalysis/ipa-alias.ll
  llvm/test/Analysis/StackSafetyAnalysis/ipa.ll
  llvm/test/LTO/Resolution/X86/alias-alias.ll
  llvm/test/LTO/Resolution/X86/comdat.ll
  llvm/test/LTO/Resolution/X86/ifunc2.ll
  llvm/test/LTO/Resolution/X86/local-def-dllimport.ll
  llvm/test/LTO/X86/Inputs/opaque-pointers.ll
  llvm/test/LTO/X86/cfi_jt_aliases.ll
  llvm/test/LTO/X86/mix-opaque-typed.ll
  llvm/test/LTO/X86/type-mapping-bug4.ll
  llvm/test/ThinLTO/X86/Inputs/import-constant.ll
  llvm/test/ThinLTO/X86/cfi-devirt.ll
  llvm/test/ThinLTO/X86/cfi-unsat.ll
  llvm/test/ThinLTO/X86/devirt-after-icp.ll
  llvm/test/ThinLTO/X86/devirt2.ll
  llvm/test/ThinLTO/X86/devirt_check.ll
  llvm/test/ThinLTO/X86/devirt_promote.ll
  llvm/test/ThinLTO/X86/devirt_single_hybrid.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-unknown.ll
  llvm/test/ThinLTO/X86/globals-import-blockaddr.ll
  llvm/test/ThinLTO/X86/import-constant.ll
  llvm/test/ThinLTO/X86/import-dsolocal.ll
  llvm/test/ThinLTO/X86/index-const-prop-gvref-pie.ll
  llvm/test/ThinLTO/X86/index-const-prop-gvref.ll
  llvm/test/ThinLTO/X86/index-const-prop-linkage.ll
  llvm/test/ThinLTO/X86/reference_non_importable.ll
  llvm/test/ThinLTO/X86/weak_externals.ll
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-lto2/llvm-lto2.cpp

Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -143,6 +143,10 @@
  cl::desc("Run PGO context sensitive IR instrumentation"),
  cl::init(false), cl::Hidden);
 
+static cl::opt LtoOpaquePointers("lto-opaque-pointers",
+   cl::desc("Enable opaque pointer types"),
+   cl::init(true), cl::Hidden);
+
 static cl::opt
 DebugPassManager("debug-pass-manager", cl::init(false), cl::Hidden,
  cl::desc("Print pass management debugging information"));
@@ -291,6 +295,7 @@
   Conf.StatsFile = StatsFile;
   Conf.PTO.LoopVectorization = Conf.OptLevel > 1;
   Conf.PTO.SLPVectorization = Conf.OptLevel > 1;
+  Conf.OpaquePointers = LtoOpaquePointers;
 
   ThinBackend Backend;
   if (ThinLTODistributedIndexes)
Index: llvm/tools/gold/gold-plugin.cpp
===
--- llvm/tools/gold/gold-plugin.cpp
+++ llvm/tools/gold/gold-plugin.cpp
@@ -208,6 +208,8 @@
   static std::string stats_file;
   // Asserts that LTO link has whole program visibility
   static bool whole_program_visibility = false;
+  // Use opaque pointer types.
+  static bool opaque_pointers = true;
 
   // Optimization remarks filename, accepted passes and hotness options
   static std::string RemarksFilename;
@@ -308,6 +310,10 @@
   RemarksFormat = std::string(opt);
 } else if (opt.consume_front("stats-file=")) {
   stats_file = std::string(opt);
+} else if (opt == "opaque-pointers") {
+  opaque_pointers = true;
+} else if (opt == "no-opaque-pointers") {
+  opaque_pointers = false;
 } else {
   // Save this option to pass to the code generator.
   // ParseCommandLineOptions() expects argv[0] to be program name. Lazily
@@ -957,6 +963,8 @@
 
   Conf.HasWholeProgramVisibility = options::whole_program_visibility;
 
+  Config.OpaquePointers = options.opaque_pointers;
+
   Conf.StatsFile = options::stats_file;
   return std::make_unique(std::move(Conf), Backend,
 options::ParallelCodeGenParallelismLevel);
Index: llvm/test/ThinLTO/X86/weak_externals.ll
===
--- llvm/test/ThinLTO/X86/weak_externals.ll
+++ llvm/test/ThinLTO/X86/weak_externals.ll
@@ -10,9 +10,9 @@
 ; RUN: llvm-dis %t.out.1.2.internalize.bc -o - | FileCheck %s --check-prefix=INTERNALIZE
 
 ; CHECK: @_ZZN9SingletonI1SE11getInstanceEvE8instance = available_externally dso_local global %struct.S zeroinitializer
-; CHECK: @_ZZN9SingletonI1SE11getInstanceEvE13instance_weak = available_externally dso_local global %struct.S* null, 

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 431853.
MatzeB marked an inline comment as done.
MatzeB added a comment.

Enable LTO by default and fix a whole bunch of LTO tests because of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/CodeGen/thinlto-inline-asm2.c
  clang/test/Driver/arm-float-abi-lto.c
  clang/test/Driver/lto-no-opaque-pointers.c
  clang/test/Driver/lto-opaque-pointers.c
  clang/test/Driver/memtag_lto.c
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/LTO.cpp
  lld/ELF/Options.td
  lld/test/ELF/lto/discard-value-names.ll
  lld/test/ELF/lto/ltopasses-basic.ll
  lld/test/ELF/lto/type-merge.ll
  lld/test/ELF/lto/type-merge2.ll
  lld/test/ELF/lto/wrap-unreferenced-before-codegen.test
  llvm/docs/OpaquePointers.rst
  llvm/include/llvm/LTO/Config.h
  llvm/test/Analysis/StackSafetyAnalysis/ipa-alias.ll
  llvm/test/Analysis/StackSafetyAnalysis/ipa.ll
  llvm/test/LTO/Resolution/X86/alias-alias.ll
  llvm/test/LTO/Resolution/X86/comdat.ll
  llvm/test/LTO/Resolution/X86/ifunc2.ll
  llvm/test/LTO/Resolution/X86/local-def-dllimport.ll
  llvm/test/LTO/X86/Inputs/opaque-pointers.ll
  llvm/test/LTO/X86/cfi_jt_aliases.ll
  llvm/test/LTO/X86/mix-opaque-typed.ll
  llvm/test/LTO/X86/type-mapping-bug4.ll
  llvm/test/ThinLTO/X86/Inputs/import-constant.ll
  llvm/test/ThinLTO/X86/cfi-devirt.ll
  llvm/test/ThinLTO/X86/cfi-unsat.ll
  llvm/test/ThinLTO/X86/devirt-after-icp.ll
  llvm/test/ThinLTO/X86/devirt2.ll
  llvm/test/ThinLTO/X86/devirt_check.ll
  llvm/test/ThinLTO/X86/devirt_promote.ll
  llvm/test/ThinLTO/X86/devirt_single_hybrid.ll
  llvm/test/ThinLTO/X86/funcattrs-prop-unknown.ll
  llvm/test/ThinLTO/X86/globals-import-blockaddr.ll
  llvm/test/ThinLTO/X86/import-constant.ll
  llvm/test/ThinLTO/X86/import-dsolocal.ll
  llvm/test/ThinLTO/X86/index-const-prop-gvref-pie.ll
  llvm/test/ThinLTO/X86/index-const-prop-gvref.ll
  llvm/test/ThinLTO/X86/index-const-prop-linkage.ll
  llvm/test/ThinLTO/X86/reference_non_importable.ll
  llvm/test/ThinLTO/X86/weak_externals.ll
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-lto2/llvm-lto2.cpp

Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -143,6 +143,10 @@
  cl::desc("Run PGO context sensitive IR instrumentation"),
  cl::init(false), cl::Hidden);
 
+static cl::opt LtoOpaquePointers("lto-opaque-pointers",
+   cl::desc("Enable opaque pointer types"),
+   cl::init(true), cl::Hidden);
+
 static cl::opt
 DebugPassManager("debug-pass-manager", cl::init(false), cl::Hidden,
  cl::desc("Print pass management debugging information"));
@@ -291,6 +295,7 @@
   Conf.StatsFile = StatsFile;
   Conf.PTO.LoopVectorization = Conf.OptLevel > 1;
   Conf.PTO.SLPVectorization = Conf.OptLevel > 1;
+  Conf.OpaquePointers = LtoOpaquePointers;
 
   ThinBackend Backend;
   if (ThinLTODistributedIndexes)
Index: llvm/tools/gold/gold-plugin.cpp
===
--- llvm/tools/gold/gold-plugin.cpp
+++ llvm/tools/gold/gold-plugin.cpp
@@ -208,6 +208,8 @@
   static std::string stats_file;
   // Asserts that LTO link has whole program visibility
   static bool whole_program_visibility = false;
+  // Use opaque pointer types.
+  static bool opaque_pointers = true;
 
   // Optimization remarks filename, accepted passes and hotness options
   static std::string RemarksFilename;
@@ -308,6 +310,10 @@
   RemarksFormat = std::string(opt);
 } else if (opt.consume_front("stats-file=")) {
   stats_file = std::string(opt);
+} else if (opt == "opaque-pointers") {
+  opaque_pointers = true;
+} else if (opt == "no-opaque-pointers") {
+  opaque_pointers = false;
 } else {
   // Save this option to pass to the code generator.
   // ParseCommandLineOptions() expects argv[0] to be program name. Lazily
@@ -957,6 +963,8 @@
 
   Conf.HasWholeProgramVisibility = options::whole_program_visibility;
 
+  Config.OpaquePointers = options.opaque_pointers;
+
   Conf.StatsFile = options::stats_file;
   return std::make_unique(std::move(Conf), Backend,
 options::ParallelCodeGenParallelismLevel);
Index: llvm/test/ThinLTO/X86/weak_externals.ll
===
--- llvm/test/ThinLTO/X86/weak_externals.ll
+++ llvm/test/ThinLTO/X86/weak_externals.ll
@@ -10,9 +10,9 @@
 ; RUN: llvm-dis %t.out.1.2.internalize.bc -o - | FileCheck %s --check-prefix=INTERNALIZE
 
 ; CHECK: @_ZZN9SingletonI1SE11getInstanceEvE8instance = available_externally dso_local global %struct.S zeroinitializer
-; CHECK: @_ZZN9SingletonI1SE11getInstanceEvE13instance_weak = 

[PATCH] D126334: Move GCC-compatible pod-packing change to v15/old behavior available at v14 and below

[Matthias Braun via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126334

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

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 430838.
Herald added subscribers: cfe-commits, arichardson.
Herald added a reviewer: MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125847

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/CodeGen/thinlto-inline-asm2.c
  clang/test/Driver/arm-float-abi-lto.c
  clang/test/Driver/lto-no-opaque-pointers.c
  clang/test/Driver/lto-opaque-pointers.c
  clang/test/Driver/memtag_lto.c
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/LTO.cpp
  lld/ELF/Options.td
  lld/test/ELF/lto/wrap-unreferenced-before-codegen.test
  llvm/docs/OpaquePointers.rst
  llvm/include/llvm/LTO/Config.h
  llvm/test/LTO/X86/Inputs/opaque-pointers.ll
  llvm/test/LTO/X86/mix-opaque-typed.ll
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-lto2/llvm-lto2.cpp

Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -143,6 +143,10 @@
  cl::desc("Run PGO context sensitive IR instrumentation"),
  cl::init(false), cl::Hidden);
 
+static cl::opt LtoOpaquePointers("lto-opaque-pointers",
+   cl::desc("Enable opaque pointer types"),
+   cl::Hidden);
+
 static cl::opt
 DebugPassManager("debug-pass-manager", cl::init(false), cl::Hidden,
  cl::desc("Print pass management debugging information"));
@@ -291,6 +295,7 @@
   Conf.StatsFile = StatsFile;
   Conf.PTO.LoopVectorization = Conf.OptLevel > 1;
   Conf.PTO.SLPVectorization = Conf.OptLevel > 1;
+  Conf.OpaquePointers = LtoOpaquePointers;
 
   ThinBackend Backend;
   if (ThinLTODistributedIndexes)
Index: llvm/tools/gold/gold-plugin.cpp
===
--- llvm/tools/gold/gold-plugin.cpp
+++ llvm/tools/gold/gold-plugin.cpp
@@ -208,6 +208,8 @@
   static std::string stats_file;
   // Asserts that LTO link has whole program visibility
   static bool whole_program_visibility = false;
+  // Use opaque pointer types.
+  static bool opaque_pointers = false;
 
   // Optimization remarks filename, accepted passes and hotness options
   static std::string RemarksFilename;
@@ -308,6 +310,10 @@
   RemarksFormat = std::string(opt);
 } else if (opt.consume_front("stats-file=")) {
   stats_file = std::string(opt);
+} else if (opt == "opaque-pointers") {
+  opaque_pointers = true;
+} else if (opt == "no-opaque-pointers") {
+  opaque_pointers = false;
 } else {
   // Save this option to pass to the code generator.
   // ParseCommandLineOptions() expects argv[0] to be program name. Lazily
@@ -957,6 +963,8 @@
 
   Conf.HasWholeProgramVisibility = options::whole_program_visibility;
 
+  Config.OpaquePointers = options.opaque_pointers;
+
   Conf.StatsFile = options::stats_file;
   return std::make_unique(std::move(Conf), Backend,
 options::ParallelCodeGenParallelismLevel);
Index: llvm/test/LTO/X86/mix-opaque-typed.ll
===
--- /dev/null
+++ llvm/test/LTO/X86/mix-opaque-typed.ll
@@ -0,0 +1,19 @@
+; RUN: llvm-as -opaque-pointers=0 %s -o %t-typed.bc
+; RUN: llvm-as -opaque-pointers=1 %S/Inputs/opaque-pointers.ll -o %t-opaque.bc
+; RUN: llvm-lto2 run -o %t-lto.bc %t-typed.bc %t-opaque.bc -save-temps \
+; RUN: -lto-opaque-pointers \
+; RUN: -r %t-typed.bc,call_foo,px -r %t-typed.bc,foo,l \
+; RUN: -r %t-opaque.bc,foo,px
+; RUN: opt -S -o - %t-lto.bc.0.4.opt.bc | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare i64 @foo(i64* %p);
+
+define i64 @call_foo(i64* %p) {
+  ; CHECK-LABEL: define i64 @call_foo(ptr nocapture readonly %p) local_unnamed_addr #0 {
+  ; CHECK-NEXT: %t.i = load i64, ptr %p, align 8
+  %t = call i64 @foo(i64* %p)
+  ret i64 %t
+}
Index: llvm/test/LTO/X86/Inputs/opaque-pointers.ll
===
--- /dev/null
+++ llvm/test/LTO/X86/Inputs/opaque-pointers.ll
@@ -0,0 +1,7 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i64 @foo(ptr %p) {
+  %t = load i64, ptr %p
+  ret i64 %t
+}
Index: llvm/include/llvm/LTO/Config.h
===
--- llvm/include/llvm/LTO/Config.h
+++ llvm/include/llvm/LTO/Config.h
@@ -177,6 +177,10 @@
   /// Add FSAFDO discriminators.
   bool AddFSDiscriminator = false;
 
+  /// Use opaque pointer types. Used to call LLVMContext::setOpaquePointers
+  /// unless already set by the `-opaque-pointers` commandline option.
+  bool OpaquePointers = false;
+
   /// If this field is set, LTO will write input file paths 

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added inline comments.
Herald added a project: All.



Comment at: clang/docs/ReleaseNotes.rst:239-243
+- GCC doesn't pack non-POD members in packed structs unless the packed
+  attribute is also specified on the member. Clang historically did perform
+  such packing. Clang now matches the gcc behavior (except on Darwin and PS4).
+  You can switch back to the old ABI behavior with the flag:
+  ``-fclang-abi-compat=13.0``.

Ugh... The release notes went missing in `main` now:

* We had them in clang-14, but then reverted in the release process of clang-14 
so they never showed for clang-14.
* In `main` we removed all release nodes when going from 14->15.

So this notice is effectively lost now...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117616

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

[PATCH] D92270: [ConstantFold] Fold more operations to poison

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added inline comments.



Comment at: llvm/lib/IR/ConstantFold.cpp:633
 // the input constant.
-return UndefValue::get(DestTy);
+return PoisonValue::get(DestTy);
   }

I believe this is causing some of our clients trouble, especially since we 
somehow have a `-fno-strict-float-cast-overflow` flag in clang that says 
float->int overflows are not UB... (CC @spatel )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92270

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

[PATCH] D51752: NFC: deduplicate isRepeatedBytePattern from clang to LLVM's isBytewiseValue

[Matthias Braun via Phabricator via cfe-commits]

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

Looks obvious and LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D51752



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

[PATCH] D51752: NFC: move isRepeatedBytePattern from clang to Constant

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

In https://reviews.llvm.org/D51752#1226462, @MatzeB wrote:

> - I assume the review lacks the changes on the llvm side?


Oops missed the link to the llvm changes in the description... At least in 
theory you should be able to make a review over all changes in parallel by 
using subversion or the experimental llvm git monorepo with 
llvm/utils/git-svn/git-llvm (admittedly haven't tried it myself yet either).


Repository:
  rC Clang

https://reviews.llvm.org/D51752



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

[PATCH] D51752: NFC: move isRepeatedBytePattern from clang to Constant

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

- I'm not a good person to review clang changes.
- I assume the review lacks the changes on the llvm side?
- On the LLVM side this feels like something for `Analysis/ValueTracking.h` in 
fact I already see `Value *isBytewiseValue(Value *V);` there, maybe that is 
good enough for your purpose?


Repository:
  rC Clang

https://reviews.llvm.org/D51752



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

[PATCH] D43108: Support for the mno-stack-arg-probe flag

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

- No test.
- What about `-mstack-arg-probe`, shouldn't we have that for consistency?
- I'd prefer not to review clang changes myself as I don't know that part of 
the code too well.


Repository:
  rC Clang

https://reviews.llvm.org/D43108



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

[PATCH] D40712: Add cc1 flag enabling the function stack size section that was added in r319430

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

Looks good to me, but I'm not too familiar with the clang driver code. So it 
would be good if you could at least add some more experienced clang developers 
into the reviewer list and wait some more days before committing.

I also wonder whether it would be possible to move the fact that it defaults to 
on for PS4 into the PS4CPU.cpp file somehow (not that things are distributed 
that cleanly right now anyway).


https://reviews.llvm.org/D40712



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

[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:477
   Context.getTypeSizeInChars(Context.getWideCharType()).getQuantity();
-  assert((LangOpts.ShortWChar ||
-  llvm::TargetLibraryInfoImpl::getTargetWCharSize(Target.getTriple()) 
==

compnerd wrote:
> compnerd wrote:
> > MatzeB wrote:
> > > rnk wrote:
> > > > @MatzeB ptal
> > > Can you find a new place for this assert()? Please do not just remove it!
> > > 
> > > For the backstory: Unfortunately I had to duplicate the wchar decision 
> > > logic inside llvm (TargetLibraryInfoImpl::getTargetWCharSize() for cases 
> > > where we just have the target triple available but need to know the size 
> > > of wchar_t using library function. This means the logic in LLVM needs to 
> > > be updated when support for new platforms is added but for people adding 
> > > platform support it will not be obvious that they have the change 
> > > LLVM/TargetLibraryInfo as well unless an assert() point them to there 
> > > being a mismatch.
> > Sure, I'll try to see if I can find a suitable place or adjustment of it.  
> > However, one thing to note is that the frontend does actually embed that 
> > into the IR metadata ("wchar_size"). 
> I think that if we try to retain this assertion, we need to update all the 
> tests to ensure that they pass the arguments for selecting the `wchar_t` 
> type.  The entire problem is that the backend view of this cannot correlate 
> with what the user specified without passing it back to it.  The "wchar_size" 
> metadata does exactly that.  Using that to perform the validation for the 
> library function call.
This has been resolved with r314187 that made the assertion obsolete.


Repository:
  rL LLVM

https://reviews.llvm.org/D37891



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

[PATCH] D29479: Driver: Do not warn about unused -pthread when linking on darwin

[Matthias Braun via Phabricator via cfe-commits]

MatzeB closed this revision.
MatzeB added a comment.

This was accepted on the mailinglist and committed in r294065


Repository:
  rL LLVM

https://reviews.llvm.org/D29479



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

[PATCH] D37562: [X86] Lower _mm[256|512]_[mask[z]]_avg_epu[8|16] intrinsics to native llvm IR

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

In https://reviews.llvm.org/D37562#877209, @craig.topper wrote:

> Was the code not using emmintrin.h and instead copied code from it that used 
> the builtins?


Turned out to be code that had been preprocessed in the past. I'll unpreprocess 
the xmmintrin.h/emmintrin.h parts now.


Repository:
  rL LLVM

https://reviews.llvm.org/D37562



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

[PATCH] D37562: [X86] Lower _mm[256|512]_[mask[z]]_avg_epu[8|16] intrinsics to native llvm IR

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

Actually, looking at the change it seems like it is and we better try to update 
emmintrin.h...


Repository:
  rL LLVM

https://reviews.llvm.org/D37562



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

[PATCH] D37562: [X86] Lower _mm[256|512]_[mask[z]]_avg_epu[8|16] intrinsics to native llvm IR

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

With this commit some (old) code stops to compile as it seems to remove some 
builtins, was that intentional?

Reproducer:

  typedef long long __m128i __attribute__((__vector_size__(16)));
  typedef short __v8hi __attribute__((__vector_size__(16)));
  typedef char __v16qi __attribute__((__vector_size__(16)));
  
  __m128i __attribute__((__always_inline__, __nodebug__))
  _mm_avg_epu8(__m128i a, __m128i b)
  {
return (__m128i)__builtin_ia32_pavgb128((__v16qi)a, (__v16qi)b);
  }
  
  __m128i __attribute__((__always_inline__, __nodebug__))
  _mm_avg_epu16(__m128i a, __m128i b)
  {
return (__m128i)__builtin_ia32_pavgw128((__v8hi)a, (__v8hi)b);
  }



  $ clang t.c
  /Users/mbraun/t.c:8:19: error: use of unknown builtin 
'__builtin_ia32_pavgb128'
[-Wimplicit-function-declaration]
return (__m128i)__builtin_ia32_pavgb128((__v16qi)a, (__v16qi)b);
^
  /Users/mbraun/t.c:8:10: error: invalid conversion between vector type 
'__m128i'
(vector of 2 'long long' values) and integer type 'int' of different 
size
return (__m128i)__builtin_ia32_pavgb128((__v16qi)a, (__v16qi)b);
   ^~~~
  /Users/mbraun/t.c:14:19: error: use of unknown builtin 
'__builtin_ia32_pavgw128'
[-Wimplicit-function-declaration]
return (__m128i)__builtin_ia32_pavgw128((__v8hi)a, (__v8hi)b);
^
  /Users/mbraun/t.c:14:10: error: invalid conversion between vector type 
'__m128i'
(vector of 2 'long long' values) and integer type 'int' of different 
size
return (__m128i)__builtin_ia32_pavgw128((__v8hi)a, (__v8hi)b);
   ^~
  4 errors generated.


Repository:
  rL LLVM

https://reviews.llvm.org/D37562



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

[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:477
   Context.getTypeSizeInChars(Context.getWideCharType()).getQuantity();
-  assert((LangOpts.ShortWChar ||
-  llvm::TargetLibraryInfoImpl::getTargetWCharSize(Target.getTriple()) 
==

rnk wrote:
> @MatzeB ptal
Can you find a new place for this assert()? Please do not just remove it!

For the backstory: Unfortunately I had to duplicate the wchar decision logic 
inside llvm (TargetLibraryInfoImpl::getTargetWCharSize() for cases where we 
just have the target triple available but need to know the size of wchar_t 
using library function. This means the logic in LLVM needs to be updated when 
support for new platforms is added but for people adding platform support it 
will not be obvious that they have the change LLVM/TargetLibraryInfo as well 
unless an assert() point them to there being a mismatch.


Repository:
  rL LLVM

https://reviews.llvm.org/D37891



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

[PATCH] D36492: [time-report] Add preprocessor timer

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added inline comments.



Comment at: lib/Lex/Preprocessor.cpp:746
 void Preprocessor::Lex(Token ) {
+  llvm::TimeRegion(PPOpts->getTimer());
+

modocache wrote:
> erik.pilkington wrote:
> > Doesn't this just start a timer and immediately end the timer? Shouldn't we 
> > do: `llvm::TimeRegion LexTime(PPOpts->getTimer())` so that the dtor gets 
> > called when this function returns and we track the time spent in this 
> > function?
> > 
> > Also: this is a pretty hot function, and it looks like TimeRegion does some 
> > non-trivial work if time is being tracked. Have you tried testing this on a 
> > big c++ file with and without this patch and seeing what the difference in 
> > compile time looks like?
> Ah, yes you're right! Sorry about that. Actually keeping the timer alive for 
> the duration of the method also reveals that the method is called 
> recursively, which causes an assert, because timers can't be started twice.
> 
> Another spot in Clang works around this with a "reference counted" timer: 
> https://github.com/llvm-mirror/clang/blob/6ac9c51ede0a50cca13dd4ac03562c036f7a3f48/lib/CodeGen/CodeGenAction.cpp#L130-L134.
>  I have a more generic version of this "reference counting timer" that I've 
> been using for some of the other timers I've been adding; maybe I'll use it 
> here as well.
FWIF: I share Eriks concerns about compiletime. Timers are enabled in optimized 
builds, and querying them is not free. So putting one into a function that is 
called a lot and is time critical seems like a bad idea (do benchmarking to 
prove or disprove this!).


https://reviews.llvm.org/D36492



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

[PATCH] D33711: [TableGen] Clang changes to support Record::getValueAsString and getValueAsListOfStrings returning StringRef instead of std::string

[Matthias Braun via Phabricator via cfe-commits]

MatzeB accepted this revision.
MatzeB added a comment.

I assume you have checked that the tablegen output doesn't change. Apart from 
that I'm all for more StringRef and SmallStrings or even Twines where possible. 
So LGTM.


https://reviews.llvm.org/D33711



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

[PATCH] D28404: IRGen: Add optnone attribute on function during O0

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

FWIW, I think this makes sense.
Moving O0 and optnone get closer seems sensible. Even though -O3 with an 
optnone function indeed gives you different results today.
We are basically maintaining two things for the same "do not optimize" goal.
This obviously won't make O0 and optnone being the same in todays pass 
managers, but it is a step in the right direction.


https://reviews.llvm.org/D28404



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

[PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

In https://reviews.llvm.org/D22045#739627, @MatzeB wrote:

> Just out of interested: I can see how `__attribute__ ((interrupt))` is 
> useful, but in what situations would you use `no_caller_saved_registers`?


Actually please answer this question by documenting the attribute in 
`docs/LangRef.rst` in the llvm part of the changes.


https://reviews.llvm.org/D22045



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

[PATCH] D22045: [X86] Support of no_caller_saved_registers attribute (Clang part)

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

Just out of interested: I can see how `__attribute__ ((interrupt))` is useful, 
but in what situations would you use `no_caller_saved_registers`?


https://reviews.llvm.org/D22045



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

[PATCH] D29479: Driver: Do not warn about unused -pthread when linking on darwin

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 87024.
MatzeB added a comment.

Address review comments:

- Simplify test
- Only perform the ClaimAll() if we actually link libc, so we get the warning 
back when combining -nostdlib/-nodefaultlibs with -pthread


Repository:
  rL LLVM

https://reviews.llvm.org/D29479

Files:
  lib/Driver/Tools.cpp
  test/Driver/darwin-ld-pthread.c


Index: test/Driver/darwin-ld-pthread.c
===
--- /dev/null
+++ test/Driver/darwin-ld-pthread.c
@@ -0,0 +1,4 @@
+// RUN: %clang -Wunused-command-line-argument -pthread -target 
x86_64-apple-darwin -### /dev/null -o %t.bin 2>&1 | FileCheck %s
+
+// There is nothing to do at link time to get pthread support. But do not warn.
+// CHECK-NOT: argument unused during compilation: '-pthread'
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -8696,6 +8696,10 @@
 
 // Let the tool chain choose which runtime library to link.
 getMachOToolChain().AddLinkRuntimeLibArgs(Args, CmdArgs);
+
+// No need to do anything for pthreads. Claim argument to avoid warning.
+Args.ClaimAllArgs(options::OPT_pthread);
+Args.ClaimAllArgs(options::OPT_pthreads);
   }
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {


Index: test/Driver/darwin-ld-pthread.c
===
--- /dev/null
+++ test/Driver/darwin-ld-pthread.c
@@ -0,0 +1,4 @@
+// RUN: %clang -Wunused-command-line-argument -pthread -target x86_64-apple-darwin -### /dev/null -o %t.bin 2>&1 | FileCheck %s
+
+// There is nothing to do at link time to get pthread support. But do not warn.
+// CHECK-NOT: argument unused during compilation: '-pthread'
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -8696,6 +8696,10 @@
 
 // Let the tool chain choose which runtime library to link.
 getMachOToolChain().AddLinkRuntimeLibArgs(Args, CmdArgs);
+
+// No need to do anything for pthreads. Claim argument to avoid warning.
+Args.ClaimAllArgs(options::OPT_pthread);
+Args.ClaimAllArgs(options::OPT_pthreads);
   }
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29479: Driver: Do not warn about unused -pthread when linking on darwin

[Matthias Braun via Phabricator via cfe-commits]

MatzeB updated this revision to Diff 87023.

Repository:
  rL LLVM

https://reviews.llvm.org/D29479

Files:
  lib/Driver/Tools.cpp
  test/Driver/darwin-ld-pthread.c


Index: test/Driver/darwin-ld-pthread.c
===
--- /dev/null
+++ test/Driver/darwin-ld-pthread.c
@@ -0,0 +1,4 @@
+// RUN: %clang -Wunused-command-line-argument -pthread -target 
x86_64-apple-darwin -### /dev/null -o %t.bin 2>&1 | FileCheck %s
+
+// There is nothing to do at link time to get pthread support. But do not warn.
+// CHECK-NOT: argument unused during compilation: '-pthread'
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -8718,6 +8718,10 @@
 }
   }
 
+  // No need to do anything for pthreads. Claim argument to avoid warning.
+  Args.ClaimAllArgs(options::OPT_pthread);
+  Args.ClaimAllArgs(options::OPT_pthreads);
+
   const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath());
   std::unique_ptr Cmd =
   llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs);


Index: test/Driver/darwin-ld-pthread.c
===
--- /dev/null
+++ test/Driver/darwin-ld-pthread.c
@@ -0,0 +1,4 @@
+// RUN: %clang -Wunused-command-line-argument -pthread -target x86_64-apple-darwin -### /dev/null -o %t.bin 2>&1 | FileCheck %s
+
+// There is nothing to do at link time to get pthread support. But do not warn.
+// CHECK-NOT: argument unused during compilation: '-pthread'
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -8718,6 +8718,10 @@
 }
   }
 
+  // No need to do anything for pthreads. Claim argument to avoid warning.
+  Args.ClaimAllArgs(options::OPT_pthread);
+  Args.ClaimAllArgs(options::OPT_pthreads);
+
   const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath());
   std::unique_ptr Cmd =
   llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29479: Driver: Do not warn about unused -pthread when linking on darwin

[Matthias Braun via Phabricator via cfe-commits]

MatzeB created this revision.
Herald added a subscriber: mcrosier.

While there is nothing to do at link time to get pthreads support on
darwin, specifying -pthread is fine and should not produce a warning
about unused arguments.


Repository:
  rL LLVM

https://reviews.llvm.org/D29479

Files:
  lib/Driver/Tools.cpp
  test/Driver/darwin-ld-pthread.c


Index: test/Driver/darwin-ld-pthread.c
===
--- /dev/null
+++ test/Driver/darwin-ld-pthread.c
@@ -0,0 +1,5 @@
+// RUN: %clang -target x86_64-apple-darwin -c -o %t.o %s
+// RUN: %clang -Wunused-command-line-argument -pthread -target 
x86_64-apple-darwin -### %t.o -o %t.bin 2>&1 | FileCheck %s
+
+// There is nothing to do at link time to get pthread support. But do not warn.
+// CHECK-NOT: argument unused during compilation: '-pthread'
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -8718,6 +8718,9 @@
 }
   }
 
+  // No need to do anything for pthreads. Claim argument to avoid warning.
+  Args.ClaimAllArgs(options::OPT_pthread);
+
   const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath());
   std::unique_ptr Cmd =
   llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs);


Index: test/Driver/darwin-ld-pthread.c
===
--- /dev/null
+++ test/Driver/darwin-ld-pthread.c
@@ -0,0 +1,5 @@
+// RUN: %clang -target x86_64-apple-darwin -c -o %t.o %s
+// RUN: %clang -Wunused-command-line-argument -pthread -target x86_64-apple-darwin -### %t.o -o %t.bin 2>&1 | FileCheck %s
+
+// There is nothing to do at link time to get pthread support. But do not warn.
+// CHECK-NOT: argument unused during compilation: '-pthread'
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -8718,6 +8718,9 @@
 }
   }
 
+  // No need to do anything for pthreads. Claim argument to avoid warning.
+  Args.ClaimAllArgs(options::OPT_pthread);
+
   const char *Exec = Args.MakeArgString(getToolChain().GetLinkerPath());
   std::unique_ptr Cmd =
   llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29476: Driver: Do not warn about unused -pthread when linking on darwin

[Matthias Braun via Phabricator via cfe-commits]

MatzeB abandoned this revision.
MatzeB added a comment.

Abandoning in favor of https://reviews.llvm.org/D29479 because phabricator...


Repository:
  rL LLVM

https://reviews.llvm.org/D29476



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

[PATCH] D29476: Driver: Do not warn about unused -pthread when linking on darwin

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a subscriber: cfe-commits.
MatzeB added a comment.

Add cfe-commits.


Repository:
  rL LLVM

https://reviews.llvm.org/D29476



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

[PATCH] D28505: CGDecl: Skip static variable initializers in unreachable code

[Matthias Braun via Phabricator via cfe-commits]

MatzeB created this revision.
MatzeB added a reviewer: rsmith.
MatzeB added a subscriber: cfe-commits.
MatzeB set the repository for this revision to rL LLVM.
Herald added a subscriber: mcrosier.

This fixes http://llvm.org/PR31054

I don't know whether that is the correct fix: Are we actually allowed to skip 
the destructor registration if the static variable is unreachable?


Repository:
  rL LLVM

https://reviews.llvm.org/D28505

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGenCXX/pr31054.cpp


Index: test/CodeGenCXX/pr31054.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pr31054.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | 
FileCheck %s
+
+struct A { ~A(); };
+void func() {
+  return;
+  static A k;
+}
+
+// Test that we did not crash, by checking whether function was created.
+// CHECK-LABEL: define void @_Z4funcv() #0 {
+// CHECK: ret void
+// CHECK: }
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -311,7 +311,7 @@
   if (!Init) {
 if (!getLangOpts().CPlusPlus)
   CGM.ErrorUnsupported(D.getInit(), "constant l-value expression");
-else if (Builder.GetInsertBlock()) {
+else if (HaveInsertPoint()) {
   // Since we have a static initializer, this global variable can't
   // be constant.
   GV->setConstant(false);
@@ -352,7 +352,7 @@
   GV->setConstant(CGM.isTypeConstant(D.getType(), true));
   GV->setInitializer(Init);
 
-  if (hasNontrivialDestruction(D.getType())) {
+  if (hasNontrivialDestruction(D.getType()) && HaveInsertPoint()) {
 // We have a constant initializer, but a nontrivial destructor. We still
 // need to perform a guarded "initialization" in order to register the
 // destructor.


Index: test/CodeGenCXX/pr31054.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pr31054.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
+
+struct A { ~A(); };
+void func() {
+  return;
+  static A k;
+}
+
+// Test that we did not crash, by checking whether function was created.
+// CHECK-LABEL: define void @_Z4funcv() #0 {
+// CHECK: ret void
+// CHECK: }
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -311,7 +311,7 @@
   if (!Init) {
 if (!getLangOpts().CPlusPlus)
   CGM.ErrorUnsupported(D.getInit(), "constant l-value expression");
-else if (Builder.GetInsertBlock()) {
+else if (HaveInsertPoint()) {
   // Since we have a static initializer, this global variable can't
   // be constant.
   GV->setConstant(false);
@@ -352,7 +352,7 @@
   GV->setConstant(CGM.isTypeConstant(D.getType(), true));
   GV->setInitializer(Init);
 
-  if (hasNontrivialDestruction(D.getType())) {
+  if (hasNontrivialDestruction(D.getType()) && HaveInsertPoint()) {
 // We have a constant initializer, but a nontrivial destructor. We still
 // need to perform a guarded "initialization" in order to register the
 // destructor.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27005: [lit] Support custom parsers in parseIntegratedTestScript

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added a comment.

This breaks the test-suite lit scripts:

Exception during script execution:
Traceback (most recent call last):

  File "/Users/mbraun/dev/public_llvm/utils/lit/lit/run.py", line 183, in 
execute_test
result = test.config.test_format.execute(test, self.lit_config)
  File "/Users/mbraun/dev/test-suite/litsupport/test.py", line 73, in execute
testfile.parse(context, test.getSourcePath())
  File "/Users/mbraun/dev/test-suite/litsupport/testfile.py", line 45, in parse
command_type,))

ValueError: unknown script command type: 'RUN:'


https://reviews.llvm.org/D27005



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