[PATCH] D131980: [Passes] Don't run tail-call-elim in -O1

2022-08-16 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 453070.
aeubanks added a comment.

delete comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131980

Files:
  clang/test/CodeGen/aarch64-ls64-inline-asm.c
  clang/test/CodeGen/aarch64-neon-vcmla.c
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.c
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abs.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_acge.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_acgt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_acle.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_aclt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_add.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adda.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_addv.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrh.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrw.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_and.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_andv.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asr.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfdot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfmlalb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfmlalt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfmmla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bic.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brka.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkn.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkpa.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkpb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cadd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clasta-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clasta.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clastb-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clastb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cls.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clz.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpeq.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpge.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpgt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmple.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmplt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpne.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpuo.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnt-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cntb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cntd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnth.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cntp.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cntw.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_compact.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create3.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create4.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cvt-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cvt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cvtnt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_div.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_divr.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dup-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dup.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_eor.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_eorv.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_expa.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ext-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ext.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_extb.c
  clang/test/CodeGen/aarch64-sve-intrinsi

[PATCH] D131980: [Passes] Don't run tail-call-elim in -O1

2022-08-16 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D131980#3726750 , @efriedma wrote:

> Are you concerned about tail-call marking, or the recursive call->loop 
> transform?

specifically tail call marking

we have symbolizers that stopped displaying some frames with this change that 
we expect to work under -O1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131980

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


[PATCH] D131986: [inlining] Add a clang option to control inlining of functions based on stack size

2022-08-16 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks accepted this revision.
aeubanks added a comment.
This revision is now accepted and ready to land.

perhaps a test that "inline-max-stacksze" doesn't appear if the flag isn't 
specified?


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

https://reviews.llvm.org/D131986

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


[PATCH] D131980: [Passes] Don't run tail-call-elim in -O1

2022-08-18 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks abandoned this revision.
aeubanks added a comment.

will use Eli's suggestion instead


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131980

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


[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

2022-08-23 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks accepted this revision.
aeubanks added a comment.
This revision is now accepted and ready to land.

sorry for the big delay, I did run the benchmarks and it looks fine

ran this through llvm-compile-time-tracker, looks good 
https://llvm-compile-time-tracker.com/compare.php?from=552b59b9e69fe1cb2b1ee0cb49cf8376a3dc0869&to=7af184006a9fd9d8deca5b7ae3625127fbb42535&stat=instructions

so I think we're good to go


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830

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


[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

2022-08-24 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

I'd try compiling with `clang -g2` to see the effects of this patch on the 
debug info in the IR


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830

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


[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

2022-08-26 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

reduced:

  define void @a() {
  entry:
%call = call float @strtof(ptr noundef null, ptr noundef null)
ret void
  }
  
  define internal float @strtof(ptr noundef %0, ptr noundef %1) nounwind {
  entry:
ret float 0.0
  }

`./build/rel/bin/opt -passes='inline,argpromotion' -disable-output /tmp/b.ll`

likely something to do with how the CGSCC pass manager handles lib function 
(see `isKnownLibFunction` in LazyCallGraph.cpp)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830

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


[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

2022-08-26 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

sent out https://reviews.llvm.org/D132764 to fix the CGSCC crash


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830

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


[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-09-22 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 462245.
aeubanks added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

"fix" lldb test (hopefully)
skip when msan
add flag to disable (to be removed in the future)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133036

Files:
  clang/test/CodeGenOpenCL/overload.cl
  lldb/test/API/functionalities/optimized_code/main.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/call-undef.ll
  llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll

Index: llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
===
--- llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
+++ llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
@@ -6,7 +6,7 @@
 ; CHECK-LABEL: @test_out_of_bounds(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:[[AND1:%.*]] = and i32 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i32 [[AND1]]
 ;
 entry:
@@ -20,7 +20,7 @@
 define i128 @test_non64bit(i128 %a) {
 ; CHECK-LABEL: @test_non64bit(
 ; CHECK-NEXT:[[AND1:%.*]] = and i128 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i128 [[AND1]]
 ;
   %and1 = and i128 %a, 3
Index: llvm/test/Transforms/InstCombine/call-undef.ll
===
--- llvm/test/Transforms/InstCombine/call-undef.ll
+++ llvm/test/Transforms/InstCombine/call-undef.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+; RUN: opt < %s -passes=instcombine -S -instcombine-disable-optimize-passing-undef-ub | FileCheck %s --check-prefix=DISABLE
 
 declare void @c(i32 noundef)
 declare void @d(ptr dereferenceable(1))
@@ -8,8 +9,12 @@
 
 define void @test1() {
 ; CHECK-LABEL: @test1(
-; CHECK-NEXT:call void @c(i32 undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test1(
+; DISABLE-NEXT:call void @c(i32 undef)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 undef)
   ret void
@@ -17,8 +22,12 @@
 
 define void @test2() {
 ; CHECK-LABEL: @test2(
-; CHECK-NEXT:call void @c(i32 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test2(
+; DISABLE-NEXT:call void @c(i32 poison)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 poison)
   ret void
@@ -26,8 +35,12 @@
 
 define void @test3() {
 ; CHECK-LABEL: @test3(
-; CHECK-NEXT:call void @e(i32 noundef undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test3(
+; DISABLE-NEXT:call void @e(i32 noundef undef)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef undef)
   ret void
@@ -35,8 +48,12 @@
 
 define void @test4() {
 ; CHECK-LABEL: @test4(
-; CHECK-NEXT:call void @e(i32 noundef poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test4(
+; DISABLE-NEXT:call void @e(i32 noundef poison)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef poison)
   ret void
@@ -44,8 +61,12 @@
 
 define void @test5() {
 ; CHECK-LABEL: @test5(
-; CHECK-NEXT:call void @d(ptr undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test5(
+; DISABLE-NEXT:call void @d(ptr undef)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr undef)
   ret void
@@ -53,8 +74,12 @@
 
 define void @test6() {
 ; CHECK-LABEL: @test6(
-; CHECK-NEXT:call void @d(ptr poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test6(
+; DISABLE-NEXT:call void @d(ptr poison)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr poison)
   ret void
@@ -62,8 +87,12 @@
 
 define void @test7() {
 ; CHECK-LABEL: @test7(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test7(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) undef)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) undef)
   ret void
@@ -71,17 +100,38 @@
 
 define void @test8() {
 ; CHECK-LABEL: @test8(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test8(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) poison)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) poison)
   ret void
 }
 
+define void @test9() sanitize_memory {
+; CHECK-LABEL: @test9(

[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-09-22 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 462247.
aeubanks added a comment.

add flag to disable (to be removed in the future)
"fix" lldb test
skip on msan instrumented functions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133036

Files:
  clang/test/CodeGenOpenCL/overload.cl
  lldb/test/API/functionalities/optimized_code/main.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/call-undef.ll
  llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll

Index: llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
===
--- llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
+++ llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
@@ -6,7 +6,7 @@
 ; CHECK-LABEL: @test_out_of_bounds(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:[[AND1:%.*]] = and i32 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i32 [[AND1]]
 ;
 entry:
@@ -20,7 +20,7 @@
 define i128 @test_non64bit(i128 %a) {
 ; CHECK-LABEL: @test_non64bit(
 ; CHECK-NEXT:[[AND1:%.*]] = and i128 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i128 [[AND1]]
 ;
   %and1 = and i128 %a, 3
Index: llvm/test/Transforms/InstCombine/call-undef.ll
===
--- llvm/test/Transforms/InstCombine/call-undef.ll
+++ llvm/test/Transforms/InstCombine/call-undef.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+; RUN: opt < %s -passes=instcombine -S -instcombine-disable-optimize-passing-undef-ub | FileCheck %s --check-prefix=DISABLE
 
 declare void @c(i32 noundef)
 declare void @d(ptr dereferenceable(1))
@@ -8,8 +9,12 @@
 
 define void @test1() {
 ; CHECK-LABEL: @test1(
-; CHECK-NEXT:call void @c(i32 undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test1(
+; DISABLE-NEXT:call void @c(i32 undef)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 undef)
   ret void
@@ -17,8 +22,12 @@
 
 define void @test2() {
 ; CHECK-LABEL: @test2(
-; CHECK-NEXT:call void @c(i32 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test2(
+; DISABLE-NEXT:call void @c(i32 poison)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 poison)
   ret void
@@ -26,8 +35,12 @@
 
 define void @test3() {
 ; CHECK-LABEL: @test3(
-; CHECK-NEXT:call void @e(i32 noundef undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test3(
+; DISABLE-NEXT:call void @e(i32 noundef undef)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef undef)
   ret void
@@ -35,8 +48,12 @@
 
 define void @test4() {
 ; CHECK-LABEL: @test4(
-; CHECK-NEXT:call void @e(i32 noundef poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test4(
+; DISABLE-NEXT:call void @e(i32 noundef poison)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef poison)
   ret void
@@ -44,8 +61,12 @@
 
 define void @test5() {
 ; CHECK-LABEL: @test5(
-; CHECK-NEXT:call void @d(ptr undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test5(
+; DISABLE-NEXT:call void @d(ptr undef)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr undef)
   ret void
@@ -53,8 +74,12 @@
 
 define void @test6() {
 ; CHECK-LABEL: @test6(
-; CHECK-NEXT:call void @d(ptr poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test6(
+; DISABLE-NEXT:call void @d(ptr poison)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr poison)
   ret void
@@ -62,8 +87,12 @@
 
 define void @test7() {
 ; CHECK-LABEL: @test7(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test7(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) undef)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) undef)
   ret void
@@ -71,17 +100,38 @@
 
 define void @test8() {
 ; CHECK-LABEL: @test8(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test8(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) poison)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) poison)
   ret void
 }
 
+define void @test9() sanitize_memory {
+; CHECK-LABEL: @test9(
+; CHECK-NEXT:call void @c(i32 undef)
+; CHECK-NEXT:r

[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-09-22 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 462249.
aeubanks added a comment.

extra parens


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133036

Files:
  clang/test/CodeGenOpenCL/overload.cl
  lldb/test/API/functionalities/optimized_code/main.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/call-undef.ll
  llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll

Index: llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
===
--- llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
+++ llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
@@ -6,7 +6,7 @@
 ; CHECK-LABEL: @test_out_of_bounds(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:[[AND1:%.*]] = and i32 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i32 [[AND1]]
 ;
 entry:
@@ -20,7 +20,7 @@
 define i128 @test_non64bit(i128 %a) {
 ; CHECK-LABEL: @test_non64bit(
 ; CHECK-NEXT:[[AND1:%.*]] = and i128 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i128 [[AND1]]
 ;
   %and1 = and i128 %a, 3
Index: llvm/test/Transforms/InstCombine/call-undef.ll
===
--- llvm/test/Transforms/InstCombine/call-undef.ll
+++ llvm/test/Transforms/InstCombine/call-undef.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+; RUN: opt < %s -passes=instcombine -S -instcombine-disable-optimize-passing-undef-ub | FileCheck %s --check-prefix=DISABLE
 
 declare void @c(i32 noundef)
 declare void @d(ptr dereferenceable(1))
@@ -8,8 +9,12 @@
 
 define void @test1() {
 ; CHECK-LABEL: @test1(
-; CHECK-NEXT:call void @c(i32 undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test1(
+; DISABLE-NEXT:call void @c(i32 undef)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 undef)
   ret void
@@ -17,8 +22,12 @@
 
 define void @test2() {
 ; CHECK-LABEL: @test2(
-; CHECK-NEXT:call void @c(i32 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test2(
+; DISABLE-NEXT:call void @c(i32 poison)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 poison)
   ret void
@@ -26,8 +35,12 @@
 
 define void @test3() {
 ; CHECK-LABEL: @test3(
-; CHECK-NEXT:call void @e(i32 noundef undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test3(
+; DISABLE-NEXT:call void @e(i32 noundef undef)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef undef)
   ret void
@@ -35,8 +48,12 @@
 
 define void @test4() {
 ; CHECK-LABEL: @test4(
-; CHECK-NEXT:call void @e(i32 noundef poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test4(
+; DISABLE-NEXT:call void @e(i32 noundef poison)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef poison)
   ret void
@@ -44,8 +61,12 @@
 
 define void @test5() {
 ; CHECK-LABEL: @test5(
-; CHECK-NEXT:call void @d(ptr undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test5(
+; DISABLE-NEXT:call void @d(ptr undef)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr undef)
   ret void
@@ -53,8 +74,12 @@
 
 define void @test6() {
 ; CHECK-LABEL: @test6(
-; CHECK-NEXT:call void @d(ptr poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test6(
+; DISABLE-NEXT:call void @d(ptr poison)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr poison)
   ret void
@@ -62,8 +87,12 @@
 
 define void @test7() {
 ; CHECK-LABEL: @test7(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test7(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) undef)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) undef)
   ret void
@@ -71,17 +100,38 @@
 
 define void @test8() {
 ; CHECK-LABEL: @test8(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test8(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) poison)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) poison)
   ret void
 }
 
+define void @test9() sanitize_memory {
+; CHECK-LABEL: @test9(
+; CHECK-NEXT:call void @c(i32 undef)
+; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test9(
+; DISABLE-NEXT:call void @c(i32 undef)
+; DISAB

[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

2022-09-22 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

that was fixed and I've relanded the patch

hopefully there aren't any more CGSCC issues this uncovers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830

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


[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-09-26 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

@vitalybuka can we turn on -fsanitize-memory-param-retval by default upstream?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133036

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


[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

2022-09-26 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks created this revision.
aeubanks added a reviewer: vitalybuka.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This eagerly reports use of undef values when passed to noundef
parameters or returned from noundef functions.

This also decreases binary sizes under msan.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134669

Files:
  clang/include/clang/Driver/Options.td
  clang/test/CodeGen/kmsan-param-retval.c
  clang/test/CodeGen/msan-param-retval.c


Index: clang/test/CodeGen/msan-param-retval.c
===
--- clang/test/CodeGen/msan-param-retval.c
+++ clang/test/CodeGen/msan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory 
-no-enable-noundef-analysis -o - %s | \
 // RUN: FileCheck %s --check-prefix=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o 
- %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory 
-fno-sanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,NOUNDEF_ONLY
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory 
-mllvm -msan-eager-checks -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory 
-no-enable-noundef-analysis -fsanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory 
-fsanitize-memory-param-retval -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o 
- %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 
 void bar(int x) {
Index: clang/test/CodeGen/kmsan-param-retval.c
===
--- clang/test/CodeGen/kmsan-param-retval.c
+++ clang/test/CodeGen/kmsan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -no-enable-noundef-analysis -o - %s | \
 // RUN: FileCheck %s --check-prefix=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -fno-sanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,NOUNDEF_ONLY
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -mllvm -msan-eager-checks -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -no-enable-noundef-analysis 
-fsanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -fsanitize-memory-param-retval -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 
 void foo();
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1767,7 +1767,7 @@
 defm sanitize_memory_param_retval
 : BoolFOption<"sanitize-memory-param-retval",
 CodeGenOpts<"SanitizeMemoryParamRetval">,
-DefaultFalse,
+DefaultTrue,
 PosFlag, NegFlag,
 BothFlags<[], " detection of uninitialized parameters and return 
values">>;
  Note: This flag was introduced when it was necessary to distinguish 
between


Index: clang/test/CodeGen/msan-param-retval.c
===
--- clang/test/CodeGen/msan-param-retval.c
+++ clang/test/CodeGen/msan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -no-enable-noundef-analysis -o - %s | \
 // RUN: FileCheck %s --check-prefix=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -fno-sanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,NOUNDEF_ONLY
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -mllvm -msan-eager-checks -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -no-enable-noundef-analysis -fsanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

2022-09-26 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

hmm, looks like this breaks a bunch of msan tests with 
`-DLLVM_ENABLE_RUNTIMES=compiler-rt` and `ninja && ninja check-runtimes`

  MemorySanitizer-X86_64 :: Linux/swapcontext_annotation_reset.cpp
  MemorySanitizer-X86_64 :: bsearch.cpp
  MemorySanitizer-X86_64 :: chained_origin.cpp
  MemorySanitizer-X86_64 :: chained_origin_empty_stack.cpp
  MemorySanitizer-X86_64 :: chained_origin_memcpy.cpp
  MemorySanitizer-X86_64 :: chained_origin_memmove.cpp
  MemorySanitizer-X86_64 :: cxa_atexit.cpp
  MemorySanitizer-X86_64 :: insertvalue_origin.cpp
  MemorySanitizer-X86_64 :: no_sanitize_memory_prop.cpp
  MemorySanitizer-X86_64 :: noundef_analysis.cpp
  MemorySanitizer-X86_64 :: param_tls_limit.cpp
  MemorySanitizer-X86_64 :: qsort.cpp
  MemorySanitizer-X86_64 :: signal_stress_test.cpp
  MemorySanitizer-X86_64 :: unpoison_param.cpp
  MemorySanitizer-X86_64 :: vector_cvt.cpp
  SanitizerCommon-msan-x86_64-Linux :: get_module_and_offset_for_pc.cpp
  UBSan-MemorySanitizer-x86_64 :: TestCases/Misc/monitor.cpp
  UBSan-MemorySanitizer-x86_64 :: TestCases/TypeCheck/vptr.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134669

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


[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-09-26 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D133036#3816002 , @aeubanks wrote:

> @vitalybuka can we turn on -fsanitize-memory-param-retval by default upstream?

attempt in https://reviews.llvm.org/D134669


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133036

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


[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

2022-09-26 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

before looking into those, is it ok to proceed with this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134669

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


[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

2022-09-26 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

do you want the tests to all properly work with -fsanitize-memory-param-retval, 
or just pin them to -fno-sanitize-memory-param-retval if they're failing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134669

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


[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

2022-09-26 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 463061.
aeubanks added a comment.

add release notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134669

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGen/kmsan-param-retval.c
  clang/test/CodeGen/msan-param-retval.c


Index: clang/test/CodeGen/msan-param-retval.c
===
--- clang/test/CodeGen/msan-param-retval.c
+++ clang/test/CodeGen/msan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory 
-no-enable-noundef-analysis -o - %s | \
 // RUN: FileCheck %s --check-prefix=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o 
- %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory 
-fno-sanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,NOUNDEF_ONLY
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory 
-mllvm -msan-eager-checks -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory 
-no-enable-noundef-analysis -fsanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory 
-fsanitize-memory-param-retval -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o 
- %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 
 void bar(int x) {
Index: clang/test/CodeGen/kmsan-param-retval.c
===
--- clang/test/CodeGen/kmsan-param-retval.c
+++ clang/test/CodeGen/kmsan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -no-enable-noundef-analysis -o - %s | \
 // RUN: FileCheck %s --check-prefix=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -fno-sanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,NOUNDEF_ONLY
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -mllvm -msan-eager-checks -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -no-enable-noundef-analysis 
-fsanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -fsanitize-memory-param-retval -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 
-fsanitize=kernel-memory -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 
 void foo();
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -1184,8 +1184,8 @@
   if (MsanUseAfterDtor)
 CmdArgs.push_back("-fsanitize-memory-use-after-dtor");
 
-  if (MsanParamRetval)
-CmdArgs.push_back("-fsanitize-memory-param-retval");
+  if (!MsanParamRetval)
+CmdArgs.push_back("-fno-sanitize-memory-param-retval");
 
   // FIXME: Pass these parameters as function attributes, not as -llvm flags.
   if (!TsanMemoryAccess) {
Index: clang/include/clang/Driver/SanitizerArgs.h
===
--- clang/include/clang/Driver/SanitizerArgs.h
+++ clang/include/clang/Driver/SanitizerArgs.h
@@ -34,7 +34,7 @@
   int BinaryMetadataFeatures = 0;
   int MsanTrackOrigins = 0;
   bool MsanUseAfterDtor = true;
-  bool MsanParamRetval = false;
+  bool MsanParamRetval = true;
   bool CfiCrossDso = false;
   bool CfiICallGeneralizePointers = false;
   bool CfiCanonicalJumpTables = false;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1767,7 +1767,7 @@
 defm sanitize_memory_param_retval
 : BoolFOption<"sanitize-memory-param-retval",
 CodeGenOpts<"SanitizeMemoryParamRetval">,
-DefaultFalse,
+DefaultTrue,
 PosFlag, NegFlag,
 BothFlags<[], " detection of uninitialized parameters and return 
values">>;
  Note: This flag was introduced when it was necessary to distinguish 
between
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNot

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

2022-09-26 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 463096.
aeubanks added a comment.
Herald added subscribers: pcwang-thead, s.egerton, simoncook.

update test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134669

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGen/kmsan-param-retval.c
  clang/test/CodeGen/msan-param-retval.c
  clang/test/Driver/fsanitize-memory-param-retval.c

Index: clang/test/Driver/fsanitize-memory-param-retval.c
===
--- clang/test/Driver/fsanitize-memory-param-retval.c
+++ clang/test/Driver/fsanitize-memory-param-retval.c
@@ -1,14 +1,14 @@
-// RUN: %clang -target i386-gnu-linux %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target aarch64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=kernel-memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target i386-gnu-linux %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target aarch64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=kernel-memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
 
-// CHECK: "-fsanitize-memory-param-retval"
+// CHECK: "-fno-sanitize-memory-param-retval"
 
-// RUN: %clang -target aarch64-linux-gnu -fsyntax-only %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck --check-prefix=11 %s
-// 11: "-fsanitize-memory-param-retval"
+// RUN: %clang -target aarch64-linux-gnu -fsyntax-only %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck --check-prefix=11 %s
+// 11: "-fno-sanitize-memory-param-retval"
 
-// RUN: not %clang -target x86_64-linux-gnu -fsyntax-only %s -fsanitize=memory -fsanitize-memory-param-retval=1 2>&1 | FileCheck --check-prefix=EXCESS %s
-// EXCESS: error: unknown argument: '-fsanitize-memory-param-retval=
+// RUN: not %clang -target x86_64-linux-gnu -fsyntax-only %s -fsanitize=memory -fno-sanitize-memory-param-retval=1 2>&1 | FileCheck --check-prefix=EXCESS %s
+// EXCESS: error: unknown argument: '-fno-sanitize-memory-param-retval=
Index: clang/test/CodeGen/msan-param-retval.c
===
--- clang/test/CodeGen/msan-param-retval.c
+++ clang/test/CodeGen/msan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -no-enable-noundef-analysis -o - %s | \
 // RUN: FileCheck %s --check-prefix=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -fno-sanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,NOUNDEF_ONLY
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -mllvm -msan-eager-checks -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -no-enable-noundef-analysis -fsanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -fsanitize-memory-param-retval -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 
 void bar(int x) {
Index: clang/test/CodeGen/kmsan-param-retval.c
===
--- clang/test/CodeGen/kmsan-param-retval.c
+++ clang/test/CodeGen/kmsan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 -fsanitize=kernel-memory -no-enable-noundef-analysis -o - %s | \
 // RUN: FileCheck %s --check-prefix=CLEAN
-// RUN: %clang_cc

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

2022-09-27 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 463322.
aeubanks added a comment.

rebase (to run through presubmits with test fixes)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134669

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGen/kmsan-param-retval.c
  clang/test/CodeGen/msan-param-retval.c
  clang/test/Driver/fsanitize-memory-param-retval.c

Index: clang/test/Driver/fsanitize-memory-param-retval.c
===
--- clang/test/Driver/fsanitize-memory-param-retval.c
+++ clang/test/Driver/fsanitize-memory-param-retval.c
@@ -1,14 +1,14 @@
-// RUN: %clang -target i386-gnu-linux %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target aarch64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=kernel-memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target i386-gnu-linux %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target aarch64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=kernel-memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
 
-// CHECK: "-fsanitize-memory-param-retval"
+// CHECK: "-fno-sanitize-memory-param-retval"
 
-// RUN: %clang -target aarch64-linux-gnu -fsyntax-only %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck --check-prefix=11 %s
-// 11: "-fsanitize-memory-param-retval"
+// RUN: %clang -target aarch64-linux-gnu -fsyntax-only %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck --check-prefix=11 %s
+// 11: "-fno-sanitize-memory-param-retval"
 
-// RUN: not %clang -target x86_64-linux-gnu -fsyntax-only %s -fsanitize=memory -fsanitize-memory-param-retval=1 2>&1 | FileCheck --check-prefix=EXCESS %s
-// EXCESS: error: unknown argument: '-fsanitize-memory-param-retval=
+// RUN: not %clang -target x86_64-linux-gnu -fsyntax-only %s -fsanitize=memory -fno-sanitize-memory-param-retval=1 2>&1 | FileCheck --check-prefix=EXCESS %s
+// EXCESS: error: unknown argument: '-fno-sanitize-memory-param-retval=
Index: clang/test/CodeGen/msan-param-retval.c
===
--- clang/test/CodeGen/msan-param-retval.c
+++ clang/test/CodeGen/msan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -no-enable-noundef-analysis -o - %s | \
 // RUN: FileCheck %s --check-prefix=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -fno-sanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,NOUNDEF_ONLY
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -mllvm -msan-eager-checks -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -no-enable-noundef-analysis -fsanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -fsanitize-memory-param-retval -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 
 void bar(int x) {
Index: clang/test/CodeGen/kmsan-param-retval.c
===
--- clang/test/CodeGen/kmsan-param-retval.c
+++ clang/test/CodeGen/kmsan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 -fsanitize=kernel-memory -no-enable-noundef-analysis -o - %s | \
 // RUN: FileCheck %s --check-prefix=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

2022-09-27 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 463327.
aeubanks added a comment.

mention -fno-sanitize-memory-param-retval in release notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134669

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGen/kmsan-param-retval.c
  clang/test/CodeGen/msan-param-retval.c
  clang/test/Driver/fsanitize-memory-param-retval.c

Index: clang/test/Driver/fsanitize-memory-param-retval.c
===
--- clang/test/Driver/fsanitize-memory-param-retval.c
+++ clang/test/Driver/fsanitize-memory-param-retval.c
@@ -1,14 +1,14 @@
-// RUN: %clang -target i386-gnu-linux %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target aarch64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=kernel-memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target i386-gnu-linux %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target aarch64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=kernel-memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
 
-// CHECK: "-fsanitize-memory-param-retval"
+// CHECK: "-fno-sanitize-memory-param-retval"
 
-// RUN: %clang -target aarch64-linux-gnu -fsyntax-only %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck --check-prefix=11 %s
-// 11: "-fsanitize-memory-param-retval"
+// RUN: %clang -target aarch64-linux-gnu -fsyntax-only %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck --check-prefix=11 %s
+// 11: "-fno-sanitize-memory-param-retval"
 
-// RUN: not %clang -target x86_64-linux-gnu -fsyntax-only %s -fsanitize=memory -fsanitize-memory-param-retval=1 2>&1 | FileCheck --check-prefix=EXCESS %s
-// EXCESS: error: unknown argument: '-fsanitize-memory-param-retval=
+// RUN: not %clang -target x86_64-linux-gnu -fsyntax-only %s -fsanitize=memory -fno-sanitize-memory-param-retval=1 2>&1 | FileCheck --check-prefix=EXCESS %s
+// EXCESS: error: unknown argument: '-fno-sanitize-memory-param-retval=
Index: clang/test/CodeGen/msan-param-retval.c
===
--- clang/test/CodeGen/msan-param-retval.c
+++ clang/test/CodeGen/msan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -no-enable-noundef-analysis -o - %s | \
 // RUN: FileCheck %s --check-prefix=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -fno-sanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,NOUNDEF_ONLY
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -mllvm -msan-eager-checks -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -no-enable-noundef-analysis -fsanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -fsanitize-memory-param-retval -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 
 void bar(int x) {
Index: clang/test/CodeGen/kmsan-param-retval.c
===
--- clang/test/CodeGen/kmsan-param-retval.c
+++ clang/test/CodeGen/kmsan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 -fsanitize=kernel-memory -no-enable-noundef-analysis -o - %s | \
 // RUN: FileCheck %s --check-prefix=CLEAN
-// RUN: %clang_cc1 -triple x86_6

[PATCH] D134669: [clang][msan] Turn on -fsanitize-memory-param-retval by default

2022-09-28 Thread Arthur Eubanks via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG44ad67031cc1: [clang][msan] Turn on 
-fsanitize-memory-param-retval by default (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134669

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGen/kmsan-param-retval.c
  clang/test/CodeGen/msan-param-retval.c
  clang/test/Driver/fsanitize-memory-param-retval.c

Index: clang/test/Driver/fsanitize-memory-param-retval.c
===
--- clang/test/Driver/fsanitize-memory-param-retval.c
+++ clang/test/Driver/fsanitize-memory-param-retval.c
@@ -1,14 +1,14 @@
-// RUN: %clang -target i386-gnu-linux %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target aarch64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target riscv32-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target riscv64-linux-gnu %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
-// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=kernel-memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target i386-gnu-linux %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target aarch64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv32-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target riscv64-linux-gnu %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
+// RUN: %clang -target x86_64-linux-gnu %s -fsanitize=kernel-memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck %s
 
-// CHECK: "-fsanitize-memory-param-retval"
+// CHECK: "-fno-sanitize-memory-param-retval"
 
-// RUN: %clang -target aarch64-linux-gnu -fsyntax-only %s -fsanitize=memory -fsanitize-memory-param-retval -c -### 2>&1 | FileCheck --check-prefix=11 %s
-// 11: "-fsanitize-memory-param-retval"
+// RUN: %clang -target aarch64-linux-gnu -fsyntax-only %s -fsanitize=memory -fno-sanitize-memory-param-retval -c -### 2>&1 | FileCheck --check-prefix=11 %s
+// 11: "-fno-sanitize-memory-param-retval"
 
-// RUN: not %clang -target x86_64-linux-gnu -fsyntax-only %s -fsanitize=memory -fsanitize-memory-param-retval=1 2>&1 | FileCheck --check-prefix=EXCESS %s
-// EXCESS: error: unknown argument: '-fsanitize-memory-param-retval=
+// RUN: not %clang -target x86_64-linux-gnu -fsyntax-only %s -fsanitize=memory -fno-sanitize-memory-param-retval=1 2>&1 | FileCheck --check-prefix=EXCESS %s
+// EXCESS: error: unknown argument: '-fno-sanitize-memory-param-retval=
Index: clang/test/CodeGen/msan-param-retval.c
===
--- clang/test/CodeGen/msan-param-retval.c
+++ clang/test/CodeGen/msan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -no-enable-noundef-analysis -o - %s | \
 // RUN: FileCheck %s --check-prefix=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -fno-sanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,NOUNDEF_ONLY
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -mllvm -msan-eager-checks -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -no-enable-noundef-analysis -fsanitize-memory-param-retval -o - %s | \
 // RUN: FileCheck %s --check-prefixes=CLEAN
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -fsanitize-memory-param-retval -o - %s | \
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -fsanitize=memory -o - %s | \
 // RUN: FileCheck %s --check-prefixes=NOUNDEF,EAGER
 
 void bar(int x) {
Index: clang/test/CodeGen/kmsan-param-retval.c
===
--- clang/test/CodeGen/kmsan-param-retval.c
+++ clang/test/CodeGen/kmsan-param-retval.c
@@ -1,12 +1,12 @@
 // RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -O2 -fsanitize=kernel-memory -no-enable-noundef-analysis -o - %s | \
 // RUN: FileC

[PATCH] D134825: [clang] Add debug info in MicrosoftCXXABI::EmitVirtualMemPtrThunk()

2022-09-28 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks created this revision.
aeubanks added a reviewer: rnk.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes https://crbug.com/1355639


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134825

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp


Index: clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -no-opaque-pointers -std=c++11 
-debug-info-kind=line-tables-only -fno-rtti -emit-llvm %s -o - 
-triple=x86_64-pc-win32 -fms-extensions | FileCheck %s
+
+struct Task {
+  virtual void Run() = 0;
+};
+
+auto b = &Task::Run;
+
+// CHECK: musttail call {{.*}}, !dbg ![[DBG:[0-9]+]]
+
+// CHECK: ![[DBG]] = !DILocation(
\ No newline at end of file
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -2083,6 +2083,8 @@
   // Start defining the function.
   CGF.StartFunction(GlobalDecl(), FnInfo.getReturnType(), ThunkFn, FnInfo,
 FunctionArgs, MD->getLocation(), SourceLocation());
+
+  ApplyDebugLocation AL(CGF, MD->getLocation());
   setCXXABIThisValue(CGF, loadIncomingCXXThis(CGF));
 
   // Load the vfptr and then callee from the vftable.  The callee should have


Index: clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -no-opaque-pointers -std=c++11 -debug-info-kind=line-tables-only -fno-rtti -emit-llvm %s -o - -triple=x86_64-pc-win32 -fms-extensions | FileCheck %s
+
+struct Task {
+  virtual void Run() = 0;
+};
+
+auto b = &Task::Run;
+
+// CHECK: musttail call {{.*}}, !dbg ![[DBG:[0-9]+]]
+
+// CHECK: ![[DBG]] = !DILocation(
\ No newline at end of file
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -2083,6 +2083,8 @@
   // Start defining the function.
   CGF.StartFunction(GlobalDecl(), FnInfo.getReturnType(), ThunkFn, FnInfo,
 FunctionArgs, MD->getLocation(), SourceLocation());
+
+  ApplyDebugLocation AL(CGF, MD->getLocation());
   setCXXABIThisValue(CGF, loadIncomingCXXThis(CGF));
 
   // Load the vfptr and then callee from the vftable.  The callee should have
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D134825: [clang] Add debug info in MicrosoftCXXABI::EmitVirtualMemPtrThunk()

2022-09-28 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 463661.
aeubanks added a comment.

update test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134825

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp


Index: clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++11 -debug-info-kind=line-tables-only -fno-rtti 
-emit-llvm %s -o - -triple=x86_64-pc-win32 -fms-extensions | FileCheck %s
+
+struct Task {
+  virtual void Run() = 0;
+};
+
+auto b = &Task::Run;
+
+// CHECK: define {{.*}}@"??_9Task@@$BA@AA"
+// CHECK-NOT: define
+// CHECK: musttail call {{.*}}, !dbg ![[DBG:[0-9]+]]
+
+// CHECK: ![[DBG]] = !DILocation(
+
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -2083,6 +2083,8 @@
   // Start defining the function.
   CGF.StartFunction(GlobalDecl(), FnInfo.getReturnType(), ThunkFn, FnInfo,
 FunctionArgs, MD->getLocation(), SourceLocation());
+
+  ApplyDebugLocation AL(CGF, MD->getLocation());
   setCXXABIThisValue(CGF, loadIncomingCXXThis(CGF));
 
   // Load the vfptr and then callee from the vftable.  The callee should have


Index: clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++11 -debug-info-kind=line-tables-only -fno-rtti -emit-llvm %s -o - -triple=x86_64-pc-win32 -fms-extensions | FileCheck %s
+
+struct Task {
+  virtual void Run() = 0;
+};
+
+auto b = &Task::Run;
+
+// CHECK: define {{.*}}@"??_9Task@@$BA@AA"
+// CHECK-NOT: define
+// CHECK: musttail call {{.*}}, !dbg ![[DBG:[0-9]+]]
+
+// CHECK: ![[DBG]] = !DILocation(
+
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -2083,6 +2083,8 @@
   // Start defining the function.
   CGF.StartFunction(GlobalDecl(), FnInfo.getReturnType(), ThunkFn, FnInfo,
 FunctionArgs, MD->getLocation(), SourceLocation());
+
+  ApplyDebugLocation AL(CGF, MD->getLocation());
   setCXXABIThisValue(CGF, loadIncomingCXXThis(CGF));
 
   // Load the vfptr and then callee from the vftable.  The callee should have
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D134825: [clang] Add debug info in MicrosoftCXXABI::EmitVirtualMemPtrThunk()

2022-09-28 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 463669.
aeubanks edited the summary of this revision.
aeubanks added a comment.

update test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134825

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp


Index: clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++11 -debug-info-kind=line-tables-only -fno-rtti 
-emit-llvm %s -o - -triple=x86_64-pc-win32 -fms-extensions | FileCheck %s
+
+struct Task {
+  virtual void Run() = 0;
+};
+
+auto b = &Task::Run;
+
+// CHECK: define {{.*}}@"??_9Task@@$BA@AA"
+// CHECK-NOT: define
+// CHECK: musttail call {{.*}}, !dbg ![[DBG:[0-9]+]]
+
+// CHECK: ![[DBG]] = !DILocation(line: 4
+
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -2083,6 +2083,8 @@
   // Start defining the function.
   CGF.StartFunction(GlobalDecl(), FnInfo.getReturnType(), ThunkFn, FnInfo,
 FunctionArgs, MD->getLocation(), SourceLocation());
+
+  ApplyDebugLocation AL(CGF, MD->getLocation());
   setCXXABIThisValue(CGF, loadIncomingCXXThis(CGF));
 
   // Load the vfptr and then callee from the vftable.  The callee should have


Index: clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++11 -debug-info-kind=line-tables-only -fno-rtti -emit-llvm %s -o - -triple=x86_64-pc-win32 -fms-extensions | FileCheck %s
+
+struct Task {
+  virtual void Run() = 0;
+};
+
+auto b = &Task::Run;
+
+// CHECK: define {{.*}}@"??_9Task@@$BA@AA"
+// CHECK-NOT: define
+// CHECK: musttail call {{.*}}, !dbg ![[DBG:[0-9]+]]
+
+// CHECK: ![[DBG]] = !DILocation(line: 4
+
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -2083,6 +2083,8 @@
   // Start defining the function.
   CGF.StartFunction(GlobalDecl(), FnInfo.getReturnType(), ThunkFn, FnInfo,
 FunctionArgs, MD->getLocation(), SourceLocation());
+
+  ApplyDebugLocation AL(CGF, MD->getLocation());
   setCXXABIThisValue(CGF, loadIncomingCXXThis(CGF));
 
   // Load the vfptr and then callee from the vftable.  The callee should have
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D134825: [clang] Add debug info in MicrosoftCXXABI::EmitVirtualMemPtrThunk()

2022-09-28 Thread Arthur Eubanks 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 rG2f3d7c2cc770: [clang] Add debug info in 
MicrosoftCXXABI::EmitVirtualMemPtrThunk() (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134825

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp


Index: clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++11 -debug-info-kind=line-tables-only -fno-rtti 
-emit-llvm %s -o - -triple=x86_64-pc-win32 -fms-extensions | FileCheck %s
+
+struct Task {
+  virtual void Run() = 0;
+};
+
+auto b = &Task::Run;
+
+// CHECK: define {{.*}}@"??_9Task@@$BA@AA"
+// CHECK-NOT: define
+// CHECK: musttail call {{.*}}, !dbg ![[DBG:[0-9]+]]
+
+// CHECK: ![[DBG]] = !DILocation(line: 4
+
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -2083,6 +2083,8 @@
   // Start defining the function.
   CGF.StartFunction(GlobalDecl(), FnInfo.getReturnType(), ThunkFn, FnInfo,
 FunctionArgs, MD->getLocation(), SourceLocation());
+
+  ApplyDebugLocation AL(CGF, MD->getLocation());
   setCXXABIThisValue(CGF, loadIncomingCXXThis(CGF));
 
   // Load the vfptr and then callee from the vftable.  The callee should have


Index: clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/microsoft-abi-member-pointers-debug-info.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++11 -debug-info-kind=line-tables-only -fno-rtti -emit-llvm %s -o - -triple=x86_64-pc-win32 -fms-extensions | FileCheck %s
+
+struct Task {
+  virtual void Run() = 0;
+};
+
+auto b = &Task::Run;
+
+// CHECK: define {{.*}}@"??_9Task@@$BA@AA"
+// CHECK-NOT: define
+// CHECK: musttail call {{.*}}, !dbg ![[DBG:[0-9]+]]
+
+// CHECK: ![[DBG]] = !DILocation(line: 4
+
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -2083,6 +2083,8 @@
   // Start defining the function.
   CGF.StartFunction(GlobalDecl(), FnInfo.getReturnType(), ThunkFn, FnInfo,
 FunctionArgs, MD->getLocation(), SourceLocation());
+
+  ApplyDebugLocation AL(CGF, MD->getLocation());
   setCXXABIThisValue(CGF, loadIncomingCXXThis(CGF));
 
   // Load the vfptr and then callee from the vftable.  The callee should have
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

2022-10-07 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks created this revision.
aeubanks added a reviewer: DavidSpickett.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

Similar to D135170 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135461

Files:
  clang/lib/AST/StmtPrinter.cpp
  lldb/test/API/lang/cpp/const_static_integral_member/main.cpp


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -35,6 +35,7 @@
   const static auto longlong_max = std::numeric_limits::max();
   const static auto ulonglong_max =
   std::numeric_limits::max();
+  const static auto wchar_max = std::numeric_limits::max();
 
   const static auto char_min = std::numeric_limits::min();
   const static auto uchar_min = std::numeric_limits::min();
@@ -45,6 +46,7 @@
   const static auto longlong_min = std::numeric_limits::min();
   const static auto ulonglong_min =
   std::numeric_limits::min();
+  const static auto wchar_min = std::numeric_limits::min();
 
   const static Enum enum_val = enum_case2;
   const static ScopedEnum scoped_enum_val = ScopedEnum::scoped_enum_case2;
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -1293,6 +1293,9 @@
 break; // no suffix.
   case BuiltinType::UInt128:
 break; // no suffix.
+  case BuiltinType::WChar_S:
+  case BuiltinType::WChar_U:
+break; // no suffix.
   }
 }
 


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -35,6 +35,7 @@
   const static auto longlong_max = std::numeric_limits::max();
   const static auto ulonglong_max =
   std::numeric_limits::max();
+  const static auto wchar_max = std::numeric_limits::max();
 
   const static auto char_min = std::numeric_limits::min();
   const static auto uchar_min = std::numeric_limits::min();
@@ -45,6 +46,7 @@
   const static auto longlong_min = std::numeric_limits::min();
   const static auto ulonglong_min =
   std::numeric_limits::min();
+  const static auto wchar_min = std::numeric_limits::min();
 
   const static Enum enum_val = enum_case2;
   const static ScopedEnum scoped_enum_val = ScopedEnum::scoped_enum_case2;
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -1293,6 +1293,9 @@
 break; // no suffix.
   case BuiltinType::UInt128:
 break; // no suffix.
+  case BuiltinType::WChar_S:
+  case BuiltinType::WChar_U:
+break; // no suffix.
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

2022-10-10 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: clang/lib/AST/StmtPrinter.cpp:1298
+  case BuiltinType::WChar_U:
+break; // no suffix.
   }

DavidSpickett wrote:
> Is there any reason to have a suffix here?
> 
> I admit this function is puzzling to me anyway given that char has a prefix 
> and this won't. Perhaps this is because char here is not "character" it's an 
> integer of a certain size. Where wide char is more about, well, being an 
> actual character.
> 
> And reading https://en.cppreference.com/w/cpp/language/character_literal the 
> suffix would be "L" which we've already used.
> 
> As long as it prints in a way that's useful for the developer that's fine.
https://en.cppreference.com/w/cpp/language/character_literal is about char 
literal prefixes, which is not what this is

e.g.
```
$ lldb a.out
(lldb) im loo -t A
...
static const unsigned char uchar_max = 255Ui8;
```
this output isn't valid C++, I believe it's just more clarification for the 
user on the exact type of the integer literal. I could make this output 
something like "UW"/"SW"?

with and without this change, we already have
```
(lldb) print A::wchar_min
(const wchar_t) $0 = L'\Ufffd'
```

this change makes the following not crash
```
$ lldb a.out
(lldb) im loo -t A
...
static const wchar_t wchar_max = 2147483647;
```
any suffix we choose here is appended to the `2147483647`



Comment at: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp:38
   std::numeric_limits::max();
+  const static auto wchar_max = std::numeric_limits::max();
 

DavidSpickett wrote:
> Is there a specific `signed wchar_t` and `unsigned wchar_t` like for char?
```
/tmp/a.cc:1:7: error: 'wchar_t' cannot be signed or unsigned 
[-Wsigned-unsigned-wchar]
const unsigned wchar_t a = 0;
```
https://en.cppreference.com/w/cpp/language/types
```
wchar_t - type for wide character representation (see wide strings). It has the 
same size, signedness, and alignment as one of the integer types, but is a 
distinct type.
```
so it sounds like under the hood the signedness is platform dependent, but 
there's only one wchar_t type


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135461

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


[PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

2022-10-10 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 466622.
aeubanks added a comment.

update python test, although those changes don't actually test the code path 
changed here, the code path here is tested via the existing `self.expect("image 
lookup -t A")`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135461

Files:
  clang/lib/AST/StmtPrinter.cpp
  
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
  lldb/test/API/lang/cpp/const_static_integral_member/main.cpp


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -36,6 +36,7 @@
   const static auto longlong_max = std::numeric_limits::max();
   const static auto ulonglong_max =
   std::numeric_limits::max();
+  const static auto wchar_max = std::numeric_limits::max();
 
   const static auto char_min = std::numeric_limits::min();
   const static auto schar_min = std::numeric_limits::min();
@@ -47,6 +48,7 @@
   const static auto longlong_min = std::numeric_limits::min();
   const static auto ulonglong_min =
   std::numeric_limits::min();
+  const static auto wchar_min = std::numeric_limits::min();
 
   const static Enum enum_val = enum_case2;
   const static ScopedEnum scoped_enum_val = ScopedEnum::scoped_enum_case2;
@@ -93,6 +95,7 @@
   auto ulong_max = A::ulong_max;
   auto longlong_max = A::longlong_max;
   auto ulonglong_max = A::ulonglong_max;
+  auto wchar_max = A::wchar_max;
 
   auto char_min = A::char_min;
   auto schar_min = A::schar_min;
@@ -103,6 +106,7 @@
   auto ulong_min = A::ulong_min;
   auto longlong_min = A::longlong_min;
   auto ulonglong_min = A::ulonglong_min;
+  auto wchar_min = A::wchar_min;
 
   int member_copy = ClassWithOnlyConstStatic::member;
 
Index: 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
===
--- 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
+++ 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
@@ -42,6 +42,7 @@
 self.expect_expr("A::ulong_max == ulong_max", result_value="true")
 self.expect_expr("A::longlong_max == longlong_max", 
result_value="true")
 self.expect_expr("A::ulonglong_max == ulonglong_max", 
result_value="true")
+self.expect_expr("A::wchar_max == wchar_max", result_value="true")
 
 self.expect_expr("A::char_min == char_min", result_value="true")
 self.expect_expr("A::schar_min == schar_min", result_value="true")
@@ -52,6 +53,7 @@
 self.expect_expr("A::ulong_min == ulong_min", result_value="true")
 self.expect_expr("A::longlong_min == longlong_min", 
result_value="true")
 self.expect_expr("A::ulonglong_min == ulonglong_min", 
result_value="true")
+self.expect_expr("A::wchar_min == wchar_min", result_value="true")
 
 # Test an unscoped enum.
 self.expect_expr("A::enum_val", result_value="enum_case2")
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -1293,6 +1293,9 @@
 break; // no suffix.
   case BuiltinType::UInt128:
 break; // no suffix.
+  case BuiltinType::WChar_S:
+  case BuiltinType::WChar_U:
+break; // no suffix
   }
 }
 


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -36,6 +36,7 @@
   const static auto longlong_max = std::numeric_limits::max();
   const static auto ulonglong_max =
   std::numeric_limits::max();
+  const static auto wchar_max = std::numeric_limits::max();
 
   const static auto char_min = std::numeric_limits::min();
   const static auto schar_min = std::numeric_limits::min();
@@ -47,6 +48,7 @@
   const static auto longlong_min = std::numeric_limits::min();
   const static auto ulonglong_min =
   std::numeric_limits::min();
+  const static auto wchar_min = std::numeric_limits::min();
 
   const static Enum enum_val = enum_case2;
   const static ScopedEnum scoped_enum_val = ScopedEnum::scoped_enum_case2;
@@ -93,6 +95,7 @@
   auto ulong_max = A::ulong_max;
   auto longlong_max = A::longlong_max;
   auto ulonglong_max = A::ulonglong_max;
+  auto wchar_max = A::wchar_max;
 
   auto char_min = A::char_min;
   auto schar_min = A::schar_min;
@@ -103,6 +106,7 @@
   auto ulong_min = A::ulong_min;
   auto longlong_min = A::longlong_min;
   auto ulonglong_min = A::ulonglong_min;
+  auto wchar_min = A::wchar_min;
 
   int member_c

[PATCH] D135461: [LLDB] Fix crash when printing a struct with a static wchar_t member

2022-10-11 Thread Arthur Eubanks via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGeab5c2f94f5a: [LLDB] Fix crash when printing a struct with a 
static wchar_t member (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135461

Files:
  clang/lib/AST/StmtPrinter.cpp
  
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
  lldb/test/API/lang/cpp/const_static_integral_member/main.cpp


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -36,6 +36,7 @@
   const static auto longlong_max = std::numeric_limits::max();
   const static auto ulonglong_max =
   std::numeric_limits::max();
+  const static auto wchar_max = std::numeric_limits::max();
 
   const static auto char_min = std::numeric_limits::min();
   const static auto schar_min = std::numeric_limits::min();
@@ -47,6 +48,7 @@
   const static auto longlong_min = std::numeric_limits::min();
   const static auto ulonglong_min =
   std::numeric_limits::min();
+  const static auto wchar_min = std::numeric_limits::min();
 
   const static Enum enum_val = enum_case2;
   const static ScopedEnum scoped_enum_val = ScopedEnum::scoped_enum_case2;
@@ -93,6 +95,7 @@
   auto ulong_max = A::ulong_max;
   auto longlong_max = A::longlong_max;
   auto ulonglong_max = A::ulonglong_max;
+  auto wchar_max = A::wchar_max;
 
   auto char_min = A::char_min;
   auto schar_min = A::schar_min;
@@ -103,6 +106,7 @@
   auto ulong_min = A::ulong_min;
   auto longlong_min = A::longlong_min;
   auto ulonglong_min = A::ulonglong_min;
+  auto wchar_min = A::wchar_min;
 
   int member_copy = ClassWithOnlyConstStatic::member;
 
Index: 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
===
--- 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
+++ 
lldb/test/API/lang/cpp/const_static_integral_member/TestConstStaticIntegralMember.py
@@ -42,6 +42,7 @@
 self.expect_expr("A::ulong_max == ulong_max", result_value="true")
 self.expect_expr("A::longlong_max == longlong_max", 
result_value="true")
 self.expect_expr("A::ulonglong_max == ulonglong_max", 
result_value="true")
+self.expect_expr("A::wchar_max == wchar_max", result_value="true")
 
 self.expect_expr("A::char_min == char_min", result_value="true")
 self.expect_expr("A::schar_min == schar_min", result_value="true")
@@ -52,6 +53,7 @@
 self.expect_expr("A::ulong_min == ulong_min", result_value="true")
 self.expect_expr("A::longlong_min == longlong_min", 
result_value="true")
 self.expect_expr("A::ulonglong_min == ulonglong_min", 
result_value="true")
+self.expect_expr("A::wchar_min == wchar_min", result_value="true")
 
 # Test an unscoped enum.
 self.expect_expr("A::enum_val", result_value="enum_case2")
Index: clang/lib/AST/StmtPrinter.cpp
===
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -1293,6 +1293,9 @@
 break; // no suffix.
   case BuiltinType::UInt128:
 break; // no suffix.
+  case BuiltinType::WChar_S:
+  case BuiltinType::WChar_U:
+break; // no suffix
   }
 }
 


Index: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
===
--- lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
+++ lldb/test/API/lang/cpp/const_static_integral_member/main.cpp
@@ -36,6 +36,7 @@
   const static auto longlong_max = std::numeric_limits::max();
   const static auto ulonglong_max =
   std::numeric_limits::max();
+  const static auto wchar_max = std::numeric_limits::max();
 
   const static auto char_min = std::numeric_limits::min();
   const static auto schar_min = std::numeric_limits::min();
@@ -47,6 +48,7 @@
   const static auto longlong_min = std::numeric_limits::min();
   const static auto ulonglong_min =
   std::numeric_limits::min();
+  const static auto wchar_min = std::numeric_limits::min();
 
   const static Enum enum_val = enum_case2;
   const static ScopedEnum scoped_enum_val = ScopedEnum::scoped_enum_case2;
@@ -93,6 +95,7 @@
   auto ulong_max = A::ulong_max;
   auto longlong_max = A::longlong_max;
   auto ulonglong_max = A::ulonglong_max;
+  auto wchar_max = A::wchar_max;
 
   auto char_min = A::char_min;
   auto schar_min = A::schar_min;
@@ -103,6 +106,7 @@
   auto ulong_min = A::ulong_min;
   auto longlong_min = A::longlong_min;
   auto ulonglong_min = A::ulonglong_min;
+  auto wchar_min = A::wchar_min;
 
   int member_copy = ClassWithOnlyConstStatic::member;
 
Index:

[PATCH] D128567: [WIP][Fuchsia] Set LLVM_TOOL_LLD_BUILD to allow some extra runtimes tests to run

2022-10-17 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

I've noticed that we're skipping compiler-rt tests with `REQUIRED: 
lld-available` because `LLVM_TOOL_LLD_BUILD` needs to be defined. any update on 
the proposed alternative?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128567

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


[PATCH] D128567: [WIP][Fuchsia] Set LLVM_TOOL_LLD_BUILD to allow some extra runtimes tests to run

2022-10-17 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D128567#3863222 , @abrachet wrote:

> In D128567#3863178 , @aeubanks 
> wrote:
>
>> I've noticed that we're skipping compiler-rt tests with `REQUIRED: 
>> lld-available` because `LLVM_TOOL_LLD_BUILD` needs to be defined. any update 
>> on the proposed alternative?
>
> I think the canonical way this is done throughout the code base is with
>
>   from lit.llvm import llvm_config
>   
>   if llvm_config.use_lld(required=False):
>   config.available_features.add('lld')
>
> Though, what I think we don't do currently here or elsewhere is conditionally 
> add test deps on `lld` if it is specified in `LLVM_ENABLE_PROJECTS`

I see things like

  if(NOT APPLE AND COMPILER_RT_HAS_LLD AND "lld" IN_LIST LLVM_ENABLE_PROJECTS)
list(APPEND ASAN_TEST_DEPS lld)
  endif()

in multiple places like `compiler-rt/test/asan/CMakeLists.txt`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128567

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


[PATCH] D136474: [CodeView][clang] Add flag to disable emitting command line into CodeView

2022-11-01 Thread Arthur Eubanks via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG29a500b346bd: [CodeView][clang] Add flag to disable emitting 
command line into CodeView (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136474

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/debug-info-codeview-buildinfo.c
  clang/test/Driver/gcodeview-command-line.c

Index: clang/test/Driver/gcodeview-command-line.c
===
--- /dev/null
+++ clang/test/Driver/gcodeview-command-line.c
@@ -0,0 +1,19 @@
+// Note: %s must be preceded by --, otherwise it may be interpreted as a
+// command-line option, e.g. on Mac where %s is commonly under /Users.
+
+// ON-NOT: "-gno-codview-commandline"
+// OFF: "-gno-codeview-command-line"
+
+// default
+// RUN: %clang_cl /Z7 -### -- %s 2>&1 | FileCheck -check-prefix=ON %s
+// enabled
+// RUN: %clang_cl /Z7 -gno-codeview-command-line -gcodeview-command-line -### -- %s 2>&1 | FileCheck -check-prefix=ON %s
+// disabled
+// RUN: %clang_cl /Z7 -gcodeview-command-line -gno-codeview-command-line -### -- %s 2>&1 | FileCheck -check-prefix=OFF %s
+
+// enabled, no /Z7
+// RUN: %clang_cl -gcodeview-command-line -### -- %s 2>&1 | FileCheck -check-prefix=ON %s
+
+// GCC-style driver
+// RUN: %clang -g -gcodeview -gno-codeview-command-line -gcodeview-command-line -### -- %s 2>&1 | FileCheck -check-prefix=ON %s
+// RUN: %clang -g -gcodeview -gcodeview-command-line -gno-codeview-command-line -### -- %s 2>&1 | FileCheck -check-prefix=OFF %s
Index: clang/test/CodeGen/debug-info-codeview-buildinfo.c
===
--- clang/test/CodeGen/debug-info-codeview-buildinfo.c
+++ clang/test/CodeGen/debug-info-codeview-buildinfo.c
@@ -1,8 +1,12 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cl --target=i686-windows-msvc /c /Z7 /Fo%t.obj -- %s
 // RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s
+// RUN: %clang_cl -gcodeview-command-line --target=i686-windows-msvc /c /Z7 /Fo%t.obj -- %s
+// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s
 // RUN: %clang_cl --target=i686-windows-msvc /c /Z7 /Fo%t.obj -fdebug-compilation-dir=. -- %s
 // RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix RELATIVE
+// RUN: %clang_cl -gno-codeview-command-line --target=i686-windows-msvc /c /Z7 /Fo%t.obj -- %s
+// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix DISABLE
 
 int main(void) { return 42; }
 
@@ -14,13 +18,21 @@
 // CHECK: 0x[[TOOL:.+]] | LF_STRING_ID [size = {{.+}}] ID: , String: [[TOOLVAL:.+[\\/]clang.*]]
 // CHECK: 0x[[CMDLINE:.+]] | LF_STRING_ID [size = {{.+}}] ID: , String: "-cc1
 // CHECK: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
-// CHECK:  0x[[PWD]]: `[[PWDVAL]]`
-// CHECK:  0x[[TOOL]]: `[[TOOLVAL]]`
-// CHECK:  0x[[FILEPATH]]: `[[FILEPATHVAL]]`
-// CHECK:  0x[[ZIPDB]]: ``
-// CHECK:  0x[[CMDLINE]]: `"-cc1
+// CHECK-NEXT:  0x[[PWD]]: `[[PWDVAL]]`
+// CHECK-NEXT:  0x[[TOOL]]: `[[TOOLVAL]]`
+// CHECK-NEXT:  0x[[FILEPATH]]: `[[FILEPATHVAL]]`
+// CHECK-NEXT:  0x[[ZIPDB]]: ``
+// CHECK-NEXT:  0x[[CMDLINE]]: `"-cc1
 
 // RELATIVE:   Types (.debug$T)
 // RELATIVE: 
 // RELATIVE: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
 // RELATIVE:  0x{{.+}}: `.`
+
+// DISABLE-NOT: cc1
+// DISABLE: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
+// DISABLE-NEXT:  0x{{.+}}: `{{.*}}`
+// DISABLE-NEXT:  : ``
+// DISABLE-NEXT:  0x{{.+}}: `{{.*}}`
+// DISABLE-NEXT:  0x{{.+}}: ``
+// DISABLE-NEXT:  : ``
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4546,8 +4546,10 @@
   }
 
   // Store the command-line for using in the CodeView backend.
-  Res.getCodeGenOpts().Argv0 = Argv0;
-  append_range(Res.getCodeGenOpts().CommandLineArgs, CommandLineArgs);
+  if (Res.getCodeGenOpts().CodeViewCommandLine) {
+Res.getCodeGenOpts().Argv0 = Argv0;
+append_range(Res.getCodeGenOpts().CommandLineArgs, CommandLineArgs);
+  }
 
   FixupInvocation(Res, Diags, Args, DashX);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4315,9 +4315,14 @@
 
   if (EmitCodeView) {
 CmdArgs.push_back("-gcodeview");
+
 Args.addOptInFlag(CmdArgs, options::OPT_gcodeview_ghash,
   options::OPT_gno_codeview_ghash);
+
+  

[PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

2022-11-07 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks created this revision.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

Followup to D134378 .

With PrintingPolicy::SuppressScope, we'd also not print the scope in template 
params. The intention was only to skip the scope for the class because we 
expect template params to be fully qualified when comparing them for simple 
template names.

Add a PrintingPolicy::NoSuppressTemplateParamsScope which disables 
SuppressScope for template params.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137583

Files:
  clang/include/clang/AST/PrettyPrinter.h
  clang/lib/AST/TypePrinter.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
  lldb/test/API/lang/cpp/unique-types2/main.cpp

Index: lldb/test/API/lang/cpp/unique-types2/main.cpp
===
--- lldb/test/API/lang/cpp/unique-types2/main.cpp
+++ lldb/test/API/lang/cpp/unique-types2/main.cpp
@@ -1,3 +1,7 @@
+namespace ns {
+struct Bar {};
+} // namespace ns
+
 template  struct Foo {
   T t;
   template  class Nested {
@@ -23,5 +27,6 @@
   FooPack p7;
 
   Foo::Nested n1;
+  Foo::Nested n2;
   // Set breakpoint here
 }
Index: lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
===
--- lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
+++ lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes.py
@@ -36,6 +36,7 @@
 self.expect("image lookup -A -t 'Foo::Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
 self.expect("image lookup -A -t 'Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
 self.expect("image lookup -A -t '::Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+self.expect("image lookup -A -t 'Foo::Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
 
 self.expect_expr("t1", result_type="Foo")
 self.expect_expr("t1", result_type="Foo")
@@ -49,6 +50,7 @@
 self.expect_expr("p6", result_type="FooPack")
 self.expect_expr("p7", result_type="FooPack")
 self.expect_expr("n1", result_type="Foo::Nested")
+self.expect_expr("n2", result_type="Foo::Nested")
 
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler_version=["<", "15.0"])
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -3792,6 +3792,7 @@
 
   clang::PrintingPolicy printing_policy(GetTypePrintingPolicy());
   printing_policy.SuppressScope = BaseOnly;
+  printing_policy.NoSuppressTemplateParamsScope = BaseOnly;
   return ConstString(qual_type.getAsString(printing_policy));
 }
 
Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -110,6 +110,23 @@
   }
 };
 
+class NoSuppressTemplateParamsScopeRAII {
+  PrintingPolicy &Policy;
+  bool OldSuppressScope;
+
+public:
+  explicit NoSuppressTemplateParamsScopeRAII(PrintingPolicy &Policy)
+  : Policy(Policy) {
+OldSuppressScope = Policy.SuppressScope;
+if (Policy.NoSuppressTemplateParamsScope)
+  Policy.SuppressScope = false;
+  }
+
+  ~NoSuppressTemplateParamsScopeRAII() {
+Policy.SuppressScope = OldSuppressScope;
+  }
+};
+
 class TypePrinter {
   PrintingPolicy Policy;
   unsigned Indentation;
@@ -1370,6 +1387,7 @@
   // If this is a class template specialization, print the template
   // arguments.
   if (const auto *Spec = dyn_cast(D)) {
+NoSuppressTemplateParamsScopeRAII SuppressScopeRAII(Policy);
 ArrayRef Args;
 TypeSourceInfo *TAW = Spec->getTypeAsWritten();
 if (!Policy.PrintCanonicalTypes && TAW) {
Index: clang/include/clang/AST/PrettyPrinter.h
===
--- clang/include/clang/AST/PrettyPrinter.h
+++ clang/include/clang/AST/PrettyPrinter.h
@@ -130,6 +130,9 @@
   /// Suppresses printing of scope specifiers.
   unsigned SuppressScope : 1;
 
+  /// When SuppressScope is true, do not apply it to template parameters.
+  unsigned NoSuppressTemplateParamsScope : 1;
+
   /// Suppress printing parts of scope specifiers that are never
   /// written, e.g., for anonymous namespaces.
   unsigned SuppressUnwrittenScope : 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137269: [Clang][AArch64][Darwin] Enable GlobalISel by default for Darwin ARM64 platforms.

2022-11-10 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D137269#3916916 , @nikic wrote:

> I'd like to object to enabling this via the frontend. This means that 
> non-clang frontends will now use a non-default configuration that is not 
> extensively tested by upstream anymore.
>
> If you don't want to change tests, you can adjust the RUN lines to explicitly 
> disable globalisel.

+1, even in-tree if you use lld/LTO you won't be using globalisel


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137269

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


[PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

2022-11-11 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 474848.
aeubanks added a comment.

use getNameForDiagnostics


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137583

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes2.py
  lldb/test/API/lang/cpp/unique-types2/main.cpp

Index: lldb/test/API/lang/cpp/unique-types2/main.cpp
===
--- lldb/test/API/lang/cpp/unique-types2/main.cpp
+++ lldb/test/API/lang/cpp/unique-types2/main.cpp
@@ -1,3 +1,7 @@
+namespace ns {
+struct Bar {};
+} // namespace ns
+
 template  struct Foo {
   T t;
   template  class Nested {
@@ -23,5 +27,6 @@
   FooPack p7;
 
   Foo::Nested n1;
+  Foo::Nested n2;
   // Set breakpoint here
 }
Index: lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes2.py
===
--- lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes2.py
+++ lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes2.py
@@ -36,6 +36,7 @@
 self.expect("image lookup -A -t 'Foo::Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
 self.expect("image lookup -A -t 'Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
 self.expect("image lookup -A -t '::Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+self.expect("image lookup -A -t 'Foo::Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
 
 self.expect_expr("t1", result_type="Foo")
 self.expect_expr("t1", result_type="Foo")
@@ -49,6 +50,7 @@
 self.expect_expr("p6", result_type="FooPack")
 self.expect_expr("p7", result_type="FooPack")
 self.expect_expr("n1", result_type="Foo::Nested")
+self.expect_expr("n2", result_type="Foo::Nested")
 
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler_version=["<", "15.0"])
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -644,7 +644,7 @@
   // Accessors
 
   ConstString GetTypeName(lldb::opaque_compiler_type_t type,
-  bool BaseOnly) override;
+  bool base_only) override;
 
   ConstString GetDisplayTypeName(lldb::opaque_compiler_type_t type) override;
 
@@ -1051,7 +1051,8 @@
   clang::PrintingPolicy GetTypePrintingPolicy();
   /// Returns the internal type name for the given NamedDecl using the
   /// type printing policy.
-  std::string GetTypeNameForDecl(const clang::NamedDecl *named_decl);
+  std::string GetTypeNameForDecl(const clang::NamedDecl *named_decl,
+ bool qualified = true);
 
   const clang::ClassTemplateSpecializationDecl *
   GetAsTemplateSpecialization(lldb::opaque_compiler_type_t type);
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2131,11 +2131,12 @@
   return printing_policy;
 }
 
-std::string TypeSystemClang::GetTypeNameForDecl(const NamedDecl *named_decl) {
+std::string TypeSystemClang::GetTypeNameForDecl(const NamedDecl *named_decl,
+bool qualified) {
   clang::PrintingPolicy printing_policy = GetTypePrintingPolicy();
   std::string result;
   llvm::raw_string_ostream os(result);
-  named_decl->printQualifiedName(os, printing_policy);
+  named_decl->getNameForDiagnostic(os, printing_policy, qualified);
   return result;
 }
 
@@ -3768,7 +3769,7 @@
 }
 
 ConstString TypeSystemClang::GetTypeName(lldb::opaque_compiler_type_t type,
- bool BaseOnly) {
+ bool base_only) {
   if (!type)
 return ConstString();
 
@@ -3790,9 +3791,9 @@
 return ConstString(GetTypeNameForDecl(typedef_decl));
   }
 
-  clang::PrintingPolicy printing_policy(GetTypePrintingPolicy());
-  printing_policy.SuppressScope = BaseOnly;
-  return ConstString(qual_type.getAsString(printing_policy));
+  if (auto *named_decl = qual_type->getAsTagDecl())
+return ConstString(GetTypeNameForDecl(named_decl, !base_only));
+  return ConstString(qual_type.getAsString(GetTypePrintingPolicy()));
 }
 
 ConstString
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

2022-11-11 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 474850.
aeubanks added a comment.

add comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137583

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes2.py
  lldb/test/API/lang/cpp/unique-types2/main.cpp

Index: lldb/test/API/lang/cpp/unique-types2/main.cpp
===
--- lldb/test/API/lang/cpp/unique-types2/main.cpp
+++ lldb/test/API/lang/cpp/unique-types2/main.cpp
@@ -1,3 +1,7 @@
+namespace ns {
+struct Bar {};
+} // namespace ns
+
 template  struct Foo {
   T t;
   template  class Nested {
@@ -23,5 +27,6 @@
   FooPack p7;
 
   Foo::Nested n1;
+  Foo::Nested n2;
   // Set breakpoint here
 }
Index: lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes2.py
===
--- lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes2.py
+++ lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes2.py
@@ -36,6 +36,7 @@
 self.expect("image lookup -A -t 'Foo::Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
 self.expect("image lookup -A -t 'Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
 self.expect("image lookup -A -t '::Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+self.expect("image lookup -A -t 'Foo::Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
 
 self.expect_expr("t1", result_type="Foo")
 self.expect_expr("t1", result_type="Foo")
@@ -49,6 +50,7 @@
 self.expect_expr("p6", result_type="FooPack")
 self.expect_expr("p7", result_type="FooPack")
 self.expect_expr("n1", result_type="Foo::Nested")
+self.expect_expr("n2", result_type="Foo::Nested")
 
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler_version=["<", "15.0"])
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -644,7 +644,7 @@
   // Accessors
 
   ConstString GetTypeName(lldb::opaque_compiler_type_t type,
-  bool BaseOnly) override;
+  bool base_only) override;
 
   ConstString GetDisplayTypeName(lldb::opaque_compiler_type_t type) override;
 
@@ -1051,7 +1051,8 @@
   clang::PrintingPolicy GetTypePrintingPolicy();
   /// Returns the internal type name for the given NamedDecl using the
   /// type printing policy.
-  std::string GetTypeNameForDecl(const clang::NamedDecl *named_decl);
+  std::string GetTypeNameForDecl(const clang::NamedDecl *named_decl,
+ bool qualified = true);
 
   const clang::ClassTemplateSpecializationDecl *
   GetAsTemplateSpecialization(lldb::opaque_compiler_type_t type);
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2131,11 +2131,12 @@
   return printing_policy;
 }
 
-std::string TypeSystemClang::GetTypeNameForDecl(const NamedDecl *named_decl) {
+std::string TypeSystemClang::GetTypeNameForDecl(const NamedDecl *named_decl,
+bool qualified) {
   clang::PrintingPolicy printing_policy = GetTypePrintingPolicy();
   std::string result;
   llvm::raw_string_ostream os(result);
-  named_decl->printQualifiedName(os, printing_policy);
+  named_decl->getNameForDiagnostic(os, printing_policy, qualified);
   return result;
 }
 
@@ -3768,7 +3769,7 @@
 }
 
 ConstString TypeSystemClang::GetTypeName(lldb::opaque_compiler_type_t type,
- bool BaseOnly) {
+ bool base_only) {
   if (!type)
 return ConstString();
 
@@ -3790,9 +3791,13 @@
 return ConstString(GetTypeNameForDecl(typedef_decl));
   }
 
-  clang::PrintingPolicy printing_policy(GetTypePrintingPolicy());
-  printing_policy.SuppressScope = BaseOnly;
-  return ConstString(qual_type.getAsString(printing_policy));
+  // For consistency, this follows the same code path that clang uses to emit
+  // debug info. This also handles when we don't want any scopes preceding the
+  // name.
+  if (auto *named_decl = qual_type->getAsTagDecl())
+return ConstString(GetTypeNameForDecl(named_decl, !base_only));
+
+  return ConstString(qual_type.getAsString(GetTypePrintingPolicy()));
 }
 
 ConstString
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

2022-11-11 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D137583#3917735 , @dblaikie wrote:

> In D137583#3917706 , @aaron.ballman 
> wrote:
>
>>> ...we expect template params to be fully qualified when comparing them for 
>>> simple template names
>>
>> So lldb is not inspecting the AST, they're doing reflection (of a sort) on 
>> the pretty printed names? Or am I misunderstanding something?
>
> Not reflection as such - but building names for the user, but partly from the 
> AST - basically LLDB wants to be able to produce the same name that 
> CGDebugInfo produces - so, maybe it should produce it the same way as 
> CGDebugInfo, which isn't to use the pretty printer from scratch.
>
> @aeubanks would this work for lldb's use case? 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGDebugInfo.cpp#L5229
>  it'd be identical to the original debug info generation, and looks like it 
> doesn't require a printing policy change/feature. Sorry I didn't think of 
> that earlier. I guess since `Qualified` would be `false` for lldb's use case, 
> you could go down into the implementation and just call the unqualified side 
> directly: `NamedDecl::printName(OS, Policy);` should print it unqualified for 
> this name, but respect the qualified printing policy flag for any nested 
> names, parameters, etc.

much better, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137583

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


[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-11-13 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 475017.
aeubanks added a comment.

I somehow missed the previous comments

rebased on top of fixing UB in tests changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133036

Files:
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/call-undef.ll
  llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll

Index: llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
===
--- llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
+++ llvm/test/Transforms/InstCombine/out-of-bounds-indexes.ll
@@ -6,7 +6,7 @@
 ; CHECK-LABEL: @test_out_of_bounds(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:[[AND1:%.*]] = and i32 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i32 [[AND1]]
 ;
 entry:
@@ -20,7 +20,7 @@
 define i128 @test_non64bit(i128 %a) {
 ; CHECK-LABEL: @test_non64bit(
 ; CHECK-NEXT:[[AND1:%.*]] = and i128 [[A:%.*]], 3
-; CHECK-NEXT:tail call void @llvm.assume(i1 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret i128 [[AND1]]
 ;
   %and1 = and i128 %a, 3
Index: llvm/test/Transforms/InstCombine/call-undef.ll
===
--- llvm/test/Transforms/InstCombine/call-undef.ll
+++ llvm/test/Transforms/InstCombine/call-undef.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+; RUN: opt < %s -passes=instcombine -S -instcombine-disable-optimize-passing-undef-ub | FileCheck %s --check-prefix=DISABLE
 
 declare void @c(i32 noundef)
 declare void @d(ptr dereferenceable(1))
@@ -8,8 +9,12 @@
 
 define void @test1() {
 ; CHECK-LABEL: @test1(
-; CHECK-NEXT:call void @c(i32 undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test1(
+; DISABLE-NEXT:call void @c(i32 undef)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 undef)
   ret void
@@ -17,8 +22,12 @@
 
 define void @test2() {
 ; CHECK-LABEL: @test2(
-; CHECK-NEXT:call void @c(i32 poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test2(
+; DISABLE-NEXT:call void @c(i32 poison)
+; DISABLE-NEXT:ret void
 ;
   call void @c(i32 poison)
   ret void
@@ -26,8 +35,12 @@
 
 define void @test3() {
 ; CHECK-LABEL: @test3(
-; CHECK-NEXT:call void @e(i32 noundef undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test3(
+; DISABLE-NEXT:call void @e(i32 noundef undef)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef undef)
   ret void
@@ -35,8 +48,12 @@
 
 define void @test4() {
 ; CHECK-LABEL: @test4(
-; CHECK-NEXT:call void @e(i32 noundef poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test4(
+; DISABLE-NEXT:call void @e(i32 noundef poison)
+; DISABLE-NEXT:ret void
 ;
   call void @e(i32 noundef poison)
   ret void
@@ -44,8 +61,12 @@
 
 define void @test5() {
 ; CHECK-LABEL: @test5(
-; CHECK-NEXT:call void @d(ptr undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test5(
+; DISABLE-NEXT:call void @d(ptr undef)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr undef)
   ret void
@@ -53,8 +74,12 @@
 
 define void @test6() {
 ; CHECK-LABEL: @test6(
-; CHECK-NEXT:call void @d(ptr poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test6(
+; DISABLE-NEXT:call void @d(ptr poison)
+; DISABLE-NEXT:ret void
 ;
   call void @d(ptr poison)
   ret void
@@ -62,8 +87,12 @@
 
 define void @test7() {
 ; CHECK-LABEL: @test7(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) undef)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test7(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) undef)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) undef)
   ret void
@@ -71,17 +100,38 @@
 
 define void @test8() {
 ; CHECK-LABEL: @test8(
-; CHECK-NEXT:call void @f(ptr dereferenceable(1) poison)
+; CHECK-NEXT:store i1 true, ptr poison, align 1
 ; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test8(
+; DISABLE-NEXT:call void @f(ptr dereferenceable(1) poison)
+; DISABLE-NEXT:ret void
 ;
   call void @f(ptr dereferenceable(1) poison)
   ret void
 }
 
+define void @test9() sanitize_memory {
+; CHECK-LABEL: @test9(
+; CHECK-NEXT:call void @c(i32 undef)
+; CHECK-NEXT:ret void
+;
+; DISABLE-LABEL: @test9(
+; DISABLE-NEXT:call void @c(i32 undef)
+; DISABLE-NEXT:ret void
+;

[PATCH] D133036: [InstCombine] Treat passing undef to noundef params as UB

2022-11-14 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

I'll hold off on submitting this until that bug is figured out


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133036

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


[PATCH] D137116: [AggressiveInstCombine] Remove legacy PM pass

2022-11-15 Thread Arthur Eubanks 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 rG70dc3b811e49: [AggressiveInstCombine] Remove legacy PM pass 
(authored by aeubanks).

Changed prior to commit:
  https://reviews.llvm.org/D137116?vs=472143&id=475583#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137116

Files:
  clang/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.cpp
  llvm/bindings/python/llvm/core.py
  llvm/include/llvm-c/Initialization.h
  llvm/include/llvm-c/Transforms/AggressiveInstCombine.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/LinkAllPasses.h
  llvm/include/llvm/Transforms/AggressiveInstCombine/AggressiveInstCombine.h
  llvm/lib/Transforms/AggressiveInstCombine/AggressiveInstCombine.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/test/Transforms/PhaseOrdering/X86/pr52078.ll
  llvm/test/Transforms/PhaseOrdering/X86/pr52253.ll
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/llvm-c-test/include-all.c
  llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp
  llvm/tools/opt/opt.cpp

Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -465,7 +465,6 @@
   initializeAnalysis(Registry);
   initializeTransformUtils(Registry);
   initializeInstCombine(Registry);
-  initializeAggressiveInstCombine(Registry);
   initializeTarget(Registry);
   // For codegen passes, only passes that do IR to IR transformation are
   // supported.
Index: llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp
===
--- llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp
+++ llvm/tools/llvm-opt-fuzzer/llvm-opt-fuzzer.cpp
@@ -197,7 +197,6 @@
   initializeAnalysis(Registry);
   initializeTransformUtils(Registry);
   initializeInstCombine(Registry);
-  initializeAggressiveInstCombine(Registry);
   initializeTarget(Registry);
 
   // Parse input options
Index: llvm/tools/llvm-c-test/include-all.c
===
--- llvm/tools/llvm-c-test/include-all.c
+++ llvm/tools/llvm-c-test/include-all.c
@@ -35,7 +35,6 @@
 #include "llvm-c/Support.h"
 #include "llvm-c/Target.h"
 #include "llvm-c/TargetMachine.h"
-#include "llvm-c/Transforms/AggressiveInstCombine.h"
 #include "llvm-c/Transforms/InstCombine.h"
 #include "llvm-c/Transforms/IPO.h"
 #include "llvm-c/Transforms/PassManagerBuilder.h"
Index: llvm/tools/bugpoint/bugpoint.cpp
===
--- llvm/tools/bugpoint/bugpoint.cpp
+++ llvm/tools/bugpoint/bugpoint.cpp
@@ -148,7 +148,6 @@
   initializeAnalysis(Registry);
   initializeTransformUtils(Registry);
   initializeInstCombine(Registry);
-  initializeAggressiveInstCombine(Registry);
   initializeTarget(Registry);
 
   if (std::getenv("bar") == (char*) -1) {
Index: llvm/test/Transforms/PhaseOrdering/X86/pr52253.ll
===
--- llvm/test/Transforms/PhaseOrdering/X86/pr52253.ll
+++ llvm/test/Transforms/PhaseOrdering/X86/pr52253.ll
@@ -1,7 +1,7 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -O3 -S < %s | FileCheck %s
-; RUN: opt -instcombine -sccp -bdce -S < %s | FileCheck %s
-; RUN: opt -aggressive-instcombine -instcombine -sccp -bdce -S < %s | FileCheck %s
+; RUN: opt -passes=instcombine,sccp,bdce -S < %s | FileCheck %s
+; RUN: opt -passes=aggressive-instcombine,instcombine,sccp,bdce -S < %s | FileCheck %s
 
 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-unknown-linux-gnu"
Index: llvm/test/Transforms/PhaseOrdering/X86/pr52078.ll
===
--- llvm/test/Transforms/PhaseOrdering/X86/pr52078.ll
+++ llvm/test/Transforms/PhaseOrdering/X86/pr52078.ll
@@ -1,7 +1,7 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -O2 -S < %s | FileCheck %s
-; RUN: opt -instcombine -S < %s | FileCheck %s --check-prefix=IC
-; RUN: opt -aggressive-instcombine -instcombine -S < %s | FileCheck %s --check-prefix=AIC_AND_IC
+; RUN: opt -passes=instcombine -S < %s | FileCheck %s --check-prefix=IC
+; RUN: opt -passes=aggressive-instcombine,instcombine -S < %s | FileCheck %s --check-prefix=AIC_AND_IC
 
 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-unknown-linux-gnu"
Index: llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
===
--- llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
+++ llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
@@ -174,8 +174,6 @@
   createCFGSimplificationPass(SimplifyCFGOptions().convertSwitchRangeToICmp(
   tr

[PATCH] D137583: [lldb] Fix simple template names and template params with scope qualifiers

2022-11-15 Thread Arthur Eubanks via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcb0ffa529a0f: [lldb] Fix simple template names and template 
params with scope qualifiers (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137583

Files:
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes2.py
  lldb/test/API/lang/cpp/unique-types2/main.cpp

Index: lldb/test/API/lang/cpp/unique-types2/main.cpp
===
--- lldb/test/API/lang/cpp/unique-types2/main.cpp
+++ lldb/test/API/lang/cpp/unique-types2/main.cpp
@@ -1,3 +1,7 @@
+namespace ns {
+struct Bar {};
+} // namespace ns
+
 template  struct Foo {
   T t;
   template  class Nested {
@@ -23,5 +27,6 @@
   FooPack p7;
 
   Foo::Nested n1;
+  Foo::Nested n2;
   // Set breakpoint here
 }
Index: lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes2.py
===
--- lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes2.py
+++ lldb/test/API/lang/cpp/unique-types2/TestUniqueTypes2.py
@@ -36,6 +36,7 @@
 self.expect("image lookup -A -t 'Foo::Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
 self.expect("image lookup -A -t 'Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
 self.expect("image lookup -A -t '::Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, error=True)
+self.expect("image lookup -A -t 'Foo::Nested'", DATA_TYPES_DISPLAYED_CORRECTLY, substrs=["1 match found"])
 
 self.expect_expr("t1", result_type="Foo")
 self.expect_expr("t1", result_type="Foo")
@@ -49,6 +50,7 @@
 self.expect_expr("p6", result_type="FooPack")
 self.expect_expr("p7", result_type="FooPack")
 self.expect_expr("n1", result_type="Foo::Nested")
+self.expect_expr("n2", result_type="Foo::Nested")
 
 @skipIf(compiler=no_match("clang"))
 @skipIf(compiler_version=["<", "15.0"])
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -644,7 +644,7 @@
   // Accessors
 
   ConstString GetTypeName(lldb::opaque_compiler_type_t type,
-  bool BaseOnly) override;
+  bool base_only) override;
 
   ConstString GetDisplayTypeName(lldb::opaque_compiler_type_t type) override;
 
@@ -1051,7 +1051,8 @@
   clang::PrintingPolicy GetTypePrintingPolicy();
   /// Returns the internal type name for the given NamedDecl using the
   /// type printing policy.
-  std::string GetTypeNameForDecl(const clang::NamedDecl *named_decl);
+  std::string GetTypeNameForDecl(const clang::NamedDecl *named_decl,
+ bool qualified = true);
 
   const clang::ClassTemplateSpecializationDecl *
   GetAsTemplateSpecialization(lldb::opaque_compiler_type_t type);
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2131,11 +2131,12 @@
   return printing_policy;
 }
 
-std::string TypeSystemClang::GetTypeNameForDecl(const NamedDecl *named_decl) {
+std::string TypeSystemClang::GetTypeNameForDecl(const NamedDecl *named_decl,
+bool qualified) {
   clang::PrintingPolicy printing_policy = GetTypePrintingPolicy();
   std::string result;
   llvm::raw_string_ostream os(result);
-  named_decl->printQualifiedName(os, printing_policy);
+  named_decl->getNameForDiagnostic(os, printing_policy, qualified);
   return result;
 }
 
@@ -3768,7 +3769,7 @@
 }
 
 ConstString TypeSystemClang::GetTypeName(lldb::opaque_compiler_type_t type,
- bool BaseOnly) {
+ bool base_only) {
   if (!type)
 return ConstString();
 
@@ -3790,9 +3791,13 @@
 return ConstString(GetTypeNameForDecl(typedef_decl));
   }
 
-  clang::PrintingPolicy printing_policy(GetTypePrintingPolicy());
-  printing_policy.SuppressScope = BaseOnly;
-  return ConstString(qual_type.getAsString(printing_policy));
+  // For consistency, this follows the same code path that clang uses to emit
+  // debug info. This also handles when we don't want any scopes preceding the
+  // name.
+  if (auto *named_decl = qual_type->getAsTagDecl())
+return ConstString(GetTypeNameForDecl(named_decl, !base_only));
+
+  return ConstString(qual_type.getAsString(GetTypePrintingPolicy()));
 }
 
 ConstString
___
cfe-commits mailing list
cfe-

[PATCH] D138081: [IR] Split out IR printing passes into IRPrinter

2022-11-17 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks accepted this revision.
aeubanks added a comment.
This revision is now accepted and ready to land.

in the description, link to the change that makes this necessary and describe 
why (need to use an analysis in the pass)




Comment at: llvm/include/llvm/IRPrinter/IRPrintingPasses.h:33
+///
+/// Note: This pass is for use with the new pass manager. Use the create...Pass
+/// functions above to create passes for use with the legacy pass manager.

obsolete (and ditto below)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138081

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


[PATCH] D138081: [IR] Split out IR printing passes into IRPrinter

2022-11-17 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: llvm/include/llvm/IRPrinter/IRPrintingPasses.h:33
+///
+/// Note: This pass is for use with the new pass manager. Use the create...Pass
+/// functions above to create passes for use with the legacy pass manager.

tejohnson wrote:
> aeubanks wrote:
> > obsolete (and ditto below)
> @aeubanks is the legacy pass manager support needed at all anymore? I'm not 
> sure of the status there.
we still need legacy PM passes in the codegen pipeline, which for sure includes 
the IR printer passes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138081

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


[PATCH] D136554: Implement CWG2631

2023-01-17 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

one more regression bisected to this: 
https://bugs.chromium.org/p/chromium/issues/detail?id=1408177
`incomplete type 'blink::ResourceClient' used in type trait expression`
I'll try to come up with a smaller repro


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

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


[PATCH] D136554: Implement CWG2631

2023-01-17 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D136554#4059723 , @aeubanks wrote:

> one more regression bisected to this: 
> https://bugs.chromium.org/p/chromium/issues/detail?id=1408177
> `incomplete type 'blink::ResourceClient' used in type trait expression`
> I'll try to come up with a smaller repro

sorry, false alarm due to faulty bisection


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

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


[PATCH] D141580: [clang] Don't consider a nullptr returned from ActOnTag() as a valid result in ParseClassSpecifier.

2023-01-17 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

apologies for reverting this, but 
https://reviews.llvm.org/rGf1f0a0d8e8fdd2e534d9423b2e64c6b8aaa53aee is broken 
and doesn't revert cleanly without this also being reverted, could you reland?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141580

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


[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

2022-06-10 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D126984#3574033 , @steplong wrote:

> In D126984#3571573 , @aeubanks 
> wrote:
>
>> IIRC in the past there was a strong preference to not have the pass manager 
>> support this sort of thing
>> if you want to support this, there should be an RFC for how the optimization 
>> part of this will work as it may require invasive changes to the LLVM pass 
>> manager
>>
>> (if this is purely a clang frontend thing then ignore me)
>
> Hmm, this does affect codegen, so I'm not sure if it's purely a clang 
> frontend thing. Maybe someone else can confirm. The motivation behind this 
> was to add support for MSVC's `pragma optimize` in D125723 
> . 
> https://docs.microsoft.com/en-us/cpp/preprocessor/optimize?view=msvc-170

adding `optsize`/`minsize`/`optnone` attributes to functions is fine and is 
already handled in optimizations, but being able to specify `-O[0-3]` would 
require a lot of new complexity in the pass manager which would likely not be 
worth it
I think D125723  is fine as is


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

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


[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

2022-06-10 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D126984#3574046 , @aaron.ballman 
wrote:

> In D126984#3571573 , @aeubanks 
> wrote:
>
>> IIRC in the past there was a strong preference to not have the pass manager 
>> support this sort of thing
>> if you want to support this, there should be an RFC for how the optimization 
>> part of this will work as it may require invasive changes to the LLVM pass 
>> manager
>>
>> (if this is purely a clang frontend thing then ignore me)
>
> Hmm, does the pass manager have to support anything here? The only Clang 
> codegen changes are for emitting IR attributes that we already emitted based 
> on command line flags/other attributes, so I had the impression this would 
> not be invasive for the backend at all.

if we're allowing individual functions to specify that they want the `-O1` 
pipeline when everything else in the module should be compiled with `-O2`, 
that's a huge change in the pass manager. but perhaps I'm misunderstanding the 
point of this patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

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


[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

2022-06-10 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D126984#3574091 , @steplong wrote:

> In D126984#3574077 , @aeubanks 
> wrote:
>
>> In D126984#3574046 , 
>> @aaron.ballman wrote:
>>
>>> In D126984#3571573 , @aeubanks 
>>> wrote:
>>>
 IIRC in the past there was a strong preference to not have the pass 
 manager support this sort of thing
 if you want to support this, there should be an RFC for how the 
 optimization part of this will work as it may require invasive changes to 
 the LLVM pass manager

 (if this is purely a clang frontend thing then ignore me)
>>>
>>> Hmm, does the pass manager have to support anything here? The only Clang 
>>> codegen changes are for emitting IR attributes that we already emitted 
>>> based on command line flags/other attributes, so I had the impression this 
>>> would not be invasive for the backend at all.
>>
>> if we're allowing individual functions to specify that they want the `-O1` 
>> pipeline when everything else in the module should be compiled with `-O2`, 
>> that's a huge change in the pass manager. but perhaps I'm misunderstanding 
>> the point of this patch
>
> That makes sense. The MSVC pragma allows 4 options, "stgy":
>
> | Parameter | On  
> | Off 
>|
> | - | 
> ---
>  |
> |
> | g | Deprecated  
> | Deprecated  
>|
> | s | Add MinSize 
> | Remove MinSize (I think this 
> would be difficult to do if -Os is passed on the cmdline) |
> | t | Add -O2 (We can't support -O2 with this attribute so ignore)
> | Add Optnone 
>|
> | y | Add frame-pointers (We can't support -f arguments with the 
> attribute in this patch so we are ignoring this) | No frame-pointers (Same 
> thing as on)   |
> |
>
> For our use case, I think we only really see `#pragma optimize("", off)` and 
> `#pragma optimize("", on)`, so I'm not opposed to abandoning this patch and 
> just supporting the common use case for now. I think `#pragma optimize("", 
> on)` would just mean do nothing and apply whatever is on the command line and 
> `#pragma optimize("", off)` would mean add optnone to the functions below it

looks good to me, I agree that we should just honor whatever optimization level 
the file is compiled with with `t`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

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


[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

2022-06-10 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

I can't speak for @xbolva00 but the only part I'm against is the user-visible 
feature of

> - Added preliminary support for GCC's attribute `optimize`, which allows 
> functions to be compiled with different optimization options than what was 
> specified on the command line.

which implies that we're on the way to support per-function optimization levels 
(which we aren't)
the internal clang representation changes are all fine

and even for the MSVC pragma `#pragma optimize("t", on)`, what are we 
supporting if the user compiles their code with `-O0`? because right now we 
won't optimize anything with `-O0`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

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


[PATCH] D126984: [clang] Add support for optimize function attribute

2022-06-13 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:3469
+support the GCC semantics. Optimization levels `-O1` through `-O4` are
+ignored. Only "-O0", "-Oz", "-Os", and "-Ofast" are supported.
+

something about `optimize(-Os)` still depending on the file's overall 
optimization level (e.g.  `clang -O0` won't do anything) would be good


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

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


[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

2022-06-15 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks created this revision.
aeubanks added reviewers: pcc, tejohnson.
Herald added subscribers: ormris, steven_wu, hiraditya.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127876

Files:
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/lto-visibility-inference.cpp

Index: clang/test/CodeGenCXX/lto-visibility-inference.cpp
===
--- clang/test/CodeGenCXX/lto-visibility-inference.cpp
+++ clang/test/CodeGenCXX/lto-visibility-inference.cpp
@@ -74,16 +74,16 @@
   // MS: type.test{{.*}}!"?AUC2@@"
   c2->f();
   // ITANIUM: type.test{{.*}}!"_ZTS2C3"
-  // MS: type.test{{.*}}!"?AUC3@@"
+  // MS-NOT: type.test{{.*}}!"?AUC3@@"
   c3->f();
   // ITANIUM: type.test{{.*}}!"_ZTS2C4"
-  // MS: type.test{{.*}}!"?AUC4@@"
+  // MS-NOT: type.test{{.*}}!"?AUC4@@"
   c4->f();
-  // ITANIUM: type.test{{.*}}!"_ZTS2C5"
-  // MS: type.test{{.*}}!"?AUC5@@"
+  // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C5"
+  // MS-NOT: type.test{{.*}}!"?AUC5@@"
   c5->f();
-  // ITANIUM: type.test{{.*}}!"_ZTS2C6"
-  // MS: type.test{{.*}}!"?AUC6@@"
+  // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C6"
+  // MS-NOT: type.test{{.*}}!"?AUC6@@"
   c6->f();
   // ITANIUM: type.test{{.*}}!"_ZTSSt2C7"
   // MS-STD: type.test{{.*}}!"?AUC7@std@@"
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -669,7 +669,7 @@
   bool ShouldEmitWPDInfo =
   CGM.getCodeGenOpts().WholeProgramVTables &&
   // Don't insert type tests if we are forcing public std visibility.
-  !CGM.HasLTOVisibilityPublicStd(RD);
+  !CGM.AlwaysHasLTOVisibilityPublic(RD);
   llvm::Value *VirtualFn = nullptr;
 
   {
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1385,7 +1385,7 @@
   /// Returns whether the given record has public std LTO visibility
   /// and therefore may not participate in (single-module) CFI and whole-program
   /// vtable optimization.
-  bool HasLTOVisibilityPublicStd(const CXXRecordDecl *RD);
+  bool AlwaysHasLTOVisibilityPublic(const CXXRecordDecl *RD);
 
   /// Returns the vcall visibility of the given type. This is the scope in which
   /// a virtual function call could be made which ends up being dispatched to a
Index: clang/lib/CodeGen/CGVTables.cpp
===
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -1175,7 +1175,15 @@
   DeferredVTables.clear();
 }
 
-bool CodeGenModule::HasLTOVisibilityPublicStd(const CXXRecordDecl *RD) {
+bool CodeGenModule::AlwaysHasLTOVisibilityPublic(const CXXRecordDecl *RD) {
+  if (RD->hasAttr() || RD->hasAttr())
+return true;
+
+  if (getTriple().isOSBinFormatCOFF()) {
+if (RD->hasAttr() || RD->hasAttr())
+  return true;
+  }
+
   if (!getCodeGenOpts().LTOVisibilityPublicStd)
 return false;
 
@@ -1200,18 +1208,12 @@
   if (!isExternallyVisible(LV.getLinkage()))
 return true;
 
-  if (RD->hasAttr() || RD->hasAttr())
-return false;
-
-  if (getTriple().isOSBinFormatCOFF()) {
-if (RD->hasAttr() || RD->hasAttr())
-  return false;
-  } else {
+  if (!getTriple().isOSBinFormatCOFF()) {
 if (LV.getVisibility() != HiddenVisibility)
   return false;
   }
 
-  return !HasLTOVisibilityPublicStd(RD);
+  return !AlwaysHasLTOVisibilityPublic(RD);
 }
 
 llvm::GlobalObject::VCallVisibility CodeGenModule::GetVCallVisibilityLevel(
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -2697,7 +2697,7 @@
   else if (CGM.getCodeGenOpts().WholeProgramVTables &&
// Don't insert type test assumes if we are forcing public std
// visibility.
-   !CGM.HasLTOVisibilityPublicStd(RD)) {
+   !CGM.AlwaysHasLTOVisibilityPublic(RD)) {
 llvm::Metadata *MD =
 CGM.CreateMetadataIdentifierForType(QualType(RD->getTypeForDecl(), 0));
 llvm::Value *TypeId =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

2022-06-15 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 437224.
aeubanks added a comment.

update some comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127876

Files:
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/lto-visibility-inference.cpp

Index: clang/test/CodeGenCXX/lto-visibility-inference.cpp
===
--- clang/test/CodeGenCXX/lto-visibility-inference.cpp
+++ clang/test/CodeGenCXX/lto-visibility-inference.cpp
@@ -74,16 +74,16 @@
   // MS: type.test{{.*}}!"?AUC2@@"
   c2->f();
   // ITANIUM: type.test{{.*}}!"_ZTS2C3"
-  // MS: type.test{{.*}}!"?AUC3@@"
+  // MS-NOT: type.test{{.*}}!"?AUC3@@"
   c3->f();
   // ITANIUM: type.test{{.*}}!"_ZTS2C4"
-  // MS: type.test{{.*}}!"?AUC4@@"
+  // MS-NOT: type.test{{.*}}!"?AUC4@@"
   c4->f();
-  // ITANIUM: type.test{{.*}}!"_ZTS2C5"
-  // MS: type.test{{.*}}!"?AUC5@@"
+  // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C5"
+  // MS-NOT: type.test{{.*}}!"?AUC5@@"
   c5->f();
-  // ITANIUM: type.test{{.*}}!"_ZTS2C6"
-  // MS: type.test{{.*}}!"?AUC6@@"
+  // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C6"
+  // MS-NOT: type.test{{.*}}!"?AUC6@@"
   c6->f();
   // ITANIUM: type.test{{.*}}!"_ZTSSt2C7"
   // MS-STD: type.test{{.*}}!"?AUC7@std@@"
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -668,8 +668,8 @@
CGM.HasHiddenLTOVisibility(RD);
   bool ShouldEmitWPDInfo =
   CGM.getCodeGenOpts().WholeProgramVTables &&
-  // Don't insert type tests if we are forcing public std visibility.
-  !CGM.HasLTOVisibilityPublicStd(RD);
+  // Don't insert type tests if we are forcing public visibility.
+  !CGM.AlwaysHasLTOVisibilityPublic(RD);
   llvm::Value *VirtualFn = nullptr;
 
   {
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1382,10 +1382,10 @@
   /// optimization.
   bool HasHiddenLTOVisibility(const CXXRecordDecl *RD);
 
-  /// Returns whether the given record has public std LTO visibility
-  /// and therefore may not participate in (single-module) CFI and whole-program
-  /// vtable optimization.
-  bool HasLTOVisibilityPublicStd(const CXXRecordDecl *RD);
+  /// Returns whether the given record has public LTO visibility (regardless of
+  /// -lto-whole-program-visibility) and therefore may not participate in
+  /// (single-module) CFI and whole-program vtable optimization.
+  bool AlwaysHasLTOVisibilityPublic(const CXXRecordDecl *RD);
 
   /// Returns the vcall visibility of the given type. This is the scope in which
   /// a virtual function call could be made which ends up being dispatched to a
Index: clang/lib/CodeGen/CGVTables.cpp
===
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -1175,7 +1175,15 @@
   DeferredVTables.clear();
 }
 
-bool CodeGenModule::HasLTOVisibilityPublicStd(const CXXRecordDecl *RD) {
+bool CodeGenModule::AlwaysHasLTOVisibilityPublic(const CXXRecordDecl *RD) {
+  if (RD->hasAttr() || RD->hasAttr())
+return true;
+
+  if (getTriple().isOSBinFormatCOFF()) {
+if (RD->hasAttr() || RD->hasAttr())
+  return true;
+  }
+
   if (!getCodeGenOpts().LTOVisibilityPublicStd)
 return false;
 
@@ -1200,18 +1208,12 @@
   if (!isExternallyVisible(LV.getLinkage()))
 return true;
 
-  if (RD->hasAttr() || RD->hasAttr())
-return false;
-
-  if (getTriple().isOSBinFormatCOFF()) {
-if (RD->hasAttr() || RD->hasAttr())
-  return false;
-  } else {
+  if (!getTriple().isOSBinFormatCOFF()) {
 if (LV.getVisibility() != HiddenVisibility)
   return false;
   }
 
-  return !HasLTOVisibilityPublicStd(RD);
+  return !AlwaysHasLTOVisibilityPublic(RD);
 }
 
 llvm::GlobalObject::VCallVisibility CodeGenModule::GetVCallVisibilityLevel(
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -2695,9 +2695,9 @@
   if (SanOpts.has(SanitizerKind::CFIVCall))
 EmitVTablePtrCheckForCall(RD, VTable, CodeGenFunction::CFITCK_VCall, Loc);
   else if (CGM.getCodeGenOpts().WholeProgramVTables &&
-   // Don't insert type test assumes if we are forcing public std
+   // Don't insert type test assumes if we are forcing public
// visibility.
-   !CGM.HasLTOVisibilityPublicStd(RD)) {
+   !CGM.AlwaysHasLTOVisibilityPublic(RD)) {
 llvm::Metadata *MD =
 CGM.CreateMetadataIdentifierForType(QualType(RD->getTypeForDecl(), 0));
 llvm::Value 

[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

2022-06-15 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

I realize that this is not a full fix for the general problem involving 
`-lto-whole-program-visibility`, but I believe if you ignore 
`-lto-whole-program-visibility` (which Chrome doesn't use) then this fix makes 
sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127876

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


[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

2022-06-15 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D127876#3586134 , @pcc wrote:

> This diverges from the documented behavior of 
> `-lto-whole-program-visibility`. The flag is meant to give all classes hidden 
> LTO visibility, but now it only does that to some of them.

perhaps I'm misunderstanding, but even with `-lto-whole-program-visibility` 
classes explicitly marked with public LTO visibility still shouldn't 
participate in WPD?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127876

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


[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

2022-06-15 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 437258.
aeubanks added a comment.

update docs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127876

Files:
  clang/docs/LTOVisibility.rst
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/lto-visibility-inference.cpp

Index: clang/test/CodeGenCXX/lto-visibility-inference.cpp
===
--- clang/test/CodeGenCXX/lto-visibility-inference.cpp
+++ clang/test/CodeGenCXX/lto-visibility-inference.cpp
@@ -79,11 +79,11 @@
   // ITANIUM: type.test{{.*}}!"_ZTS2C4"
   // MS: type.test{{.*}}!"?AUC4@@"
   c4->f();
-  // ITANIUM: type.test{{.*}}!"_ZTS2C5"
-  // MS: type.test{{.*}}!"?AUC5@@"
+  // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C5"
+  // MS-NOT: type.test{{.*}}!"?AUC5@@"
   c5->f();
-  // ITANIUM: type.test{{.*}}!"_ZTS2C6"
-  // MS: type.test{{.*}}!"?AUC6@@"
+  // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C6"
+  // MS-NOT: type.test{{.*}}!"?AUC6@@"
   c6->f();
   // ITANIUM: type.test{{.*}}!"_ZTSSt2C7"
   // MS-STD: type.test{{.*}}!"?AUC7@std@@"
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -668,8 +668,8 @@
CGM.HasHiddenLTOVisibility(RD);
   bool ShouldEmitWPDInfo =
   CGM.getCodeGenOpts().WholeProgramVTables &&
-  // Don't insert type tests if we are forcing public std visibility.
-  !CGM.HasLTOVisibilityPublicStd(RD);
+  // Don't insert type tests if we are forcing public visibility.
+  !CGM.AlwaysHasLTOVisibilityPublic(RD);
   llvm::Value *VirtualFn = nullptr;
 
   {
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1382,10 +1382,10 @@
   /// optimization.
   bool HasHiddenLTOVisibility(const CXXRecordDecl *RD);
 
-  /// Returns whether the given record has public std LTO visibility
-  /// and therefore may not participate in (single-module) CFI and whole-program
-  /// vtable optimization.
-  bool HasLTOVisibilityPublicStd(const CXXRecordDecl *RD);
+  /// Returns whether the given record has public LTO visibility (regardless of
+  /// -lto-whole-program-visibility) and therefore may not participate in
+  /// (single-module) CFI and whole-program vtable optimization.
+  bool AlwaysHasLTOVisibilityPublic(const CXXRecordDecl *RD);
 
   /// Returns the vcall visibility of the given type. This is the scope in which
   /// a virtual function call could be made which ends up being dispatched to a
Index: clang/lib/CodeGen/CGVTables.cpp
===
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -1175,7 +1175,10 @@
   DeferredVTables.clear();
 }
 
-bool CodeGenModule::HasLTOVisibilityPublicStd(const CXXRecordDecl *RD) {
+bool CodeGenModule::AlwaysHasLTOVisibilityPublic(const CXXRecordDecl *RD) {
+  if (RD->hasAttr() || RD->hasAttr())
+return true;
+
   if (!getCodeGenOpts().LTOVisibilityPublicStd)
 return false;
 
@@ -1200,9 +1203,6 @@
   if (!isExternallyVisible(LV.getLinkage()))
 return true;
 
-  if (RD->hasAttr() || RD->hasAttr())
-return false;
-
   if (getTriple().isOSBinFormatCOFF()) {
 if (RD->hasAttr() || RD->hasAttr())
   return false;
@@ -1211,7 +1211,7 @@
   return false;
   }
 
-  return !HasLTOVisibilityPublicStd(RD);
+  return !AlwaysHasLTOVisibilityPublic(RD);
 }
 
 llvm::GlobalObject::VCallVisibility CodeGenModule::GetVCallVisibilityLevel(
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -2695,9 +2695,9 @@
   if (SanOpts.has(SanitizerKind::CFIVCall))
 EmitVTablePtrCheckForCall(RD, VTable, CodeGenFunction::CFITCK_VCall, Loc);
   else if (CGM.getCodeGenOpts().WholeProgramVTables &&
-   // Don't insert type test assumes if we are forcing public std
+   // Don't insert type test assumes if we are forcing public
// visibility.
-   !CGM.HasLTOVisibilityPublicStd(RD)) {
+   !CGM.AlwaysHasLTOVisibilityPublic(RD)) {
 llvm::Metadata *MD =
 CGM.CreateMetadataIdentifierForType(QualType(RD->getTypeForDecl(), 0));
 llvm::Value *TypeId =
Index: clang/docs/LTOVisibility.rst
===
--- clang/docs/LTOVisibility.rst
+++ clang/docs/LTOVisibility.rst
@@ -35,15 +35,16 @@
 (e.g. classes declared in unnamed namespaces) also receive hidden LTO
 visibility.
 
-During the LTO link, all classes with public LTO visibility will be refined
-to hidden LTO visibility 

[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

2022-06-15 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

> We documented the feature in D75655  and 
> there it says "all classes" (and still does).

I've updated the documentation. It was already slightly inaccurate in that we 
weren't emitting type test/assumes for `std`/`stdext` namespace classes when 
`-flto-visibility-public-std`, this now corrects that part and generalizes that 
part of the documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127876

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


[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

2022-06-15 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks updated this revision to Diff 437420.
aeubanks added a comment.

update docs to only mention `[[clang::lto_visibility_public]]`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127876

Files:
  clang/docs/LTOVisibility.rst
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/lto-visibility-inference.cpp

Index: clang/test/CodeGenCXX/lto-visibility-inference.cpp
===
--- clang/test/CodeGenCXX/lto-visibility-inference.cpp
+++ clang/test/CodeGenCXX/lto-visibility-inference.cpp
@@ -79,11 +79,11 @@
   // ITANIUM: type.test{{.*}}!"_ZTS2C4"
   // MS: type.test{{.*}}!"?AUC4@@"
   c4->f();
-  // ITANIUM: type.test{{.*}}!"_ZTS2C5"
-  // MS: type.test{{.*}}!"?AUC5@@"
+  // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C5"
+  // MS-NOT: type.test{{.*}}!"?AUC5@@"
   c5->f();
-  // ITANIUM: type.test{{.*}}!"_ZTS2C6"
-  // MS: type.test{{.*}}!"?AUC6@@"
+  // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C6"
+  // MS-NOT: type.test{{.*}}!"?AUC6@@"
   c6->f();
   // ITANIUM: type.test{{.*}}!"_ZTSSt2C7"
   // MS-STD: type.test{{.*}}!"?AUC7@std@@"
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -668,8 +668,8 @@
CGM.HasHiddenLTOVisibility(RD);
   bool ShouldEmitWPDInfo =
   CGM.getCodeGenOpts().WholeProgramVTables &&
-  // Don't insert type tests if we are forcing public std visibility.
-  !CGM.HasLTOVisibilityPublicStd(RD);
+  // Don't insert type tests if we are forcing public visibility.
+  !CGM.AlwaysHasLTOVisibilityPublic(RD);
   llvm::Value *VirtualFn = nullptr;
 
   {
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1382,10 +1382,10 @@
   /// optimization.
   bool HasHiddenLTOVisibility(const CXXRecordDecl *RD);
 
-  /// Returns whether the given record has public std LTO visibility
-  /// and therefore may not participate in (single-module) CFI and whole-program
-  /// vtable optimization.
-  bool HasLTOVisibilityPublicStd(const CXXRecordDecl *RD);
+  /// Returns whether the given record has public LTO visibility (regardless of
+  /// -lto-whole-program-visibility) and therefore may not participate in
+  /// (single-module) CFI and whole-program vtable optimization.
+  bool AlwaysHasLTOVisibilityPublic(const CXXRecordDecl *RD);
 
   /// Returns the vcall visibility of the given type. This is the scope in which
   /// a virtual function call could be made which ends up being dispatched to a
Index: clang/lib/CodeGen/CGVTables.cpp
===
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -1175,7 +1175,10 @@
   DeferredVTables.clear();
 }
 
-bool CodeGenModule::HasLTOVisibilityPublicStd(const CXXRecordDecl *RD) {
+bool CodeGenModule::AlwaysHasLTOVisibilityPublic(const CXXRecordDecl *RD) {
+  if (RD->hasAttr() || RD->hasAttr())
+return true;
+
   if (!getCodeGenOpts().LTOVisibilityPublicStd)
 return false;
 
@@ -1200,9 +1203,6 @@
   if (!isExternallyVisible(LV.getLinkage()))
 return true;
 
-  if (RD->hasAttr() || RD->hasAttr())
-return false;
-
   if (getTriple().isOSBinFormatCOFF()) {
 if (RD->hasAttr() || RD->hasAttr())
   return false;
@@ -1211,7 +1211,7 @@
   return false;
   }
 
-  return !HasLTOVisibilityPublicStd(RD);
+  return !AlwaysHasLTOVisibilityPublic(RD);
 }
 
 llvm::GlobalObject::VCallVisibility CodeGenModule::GetVCallVisibilityLevel(
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -2695,9 +2695,9 @@
   if (SanOpts.has(SanitizerKind::CFIVCall))
 EmitVTablePtrCheckForCall(RD, VTable, CodeGenFunction::CFITCK_VCall, Loc);
   else if (CGM.getCodeGenOpts().WholeProgramVTables &&
-   // Don't insert type test assumes if we are forcing public std
+   // Don't insert type test assumes if we are forcing public
// visibility.
-   !CGM.HasLTOVisibilityPublicStd(RD)) {
+   !CGM.AlwaysHasLTOVisibilityPublic(RD)) {
 llvm::Metadata *MD =
 CGM.CreateMetadataIdentifierForType(QualType(RD->getTypeForDecl(), 0));
 llvm::Value *TypeId =
Index: clang/docs/LTOVisibility.rst
===
--- clang/docs/LTOVisibility.rst
+++ clang/docs/LTOVisibility.rst
@@ -35,13 +35,14 @@
 (e.g. classes declared in unnamed namespaces) also receive hidden LTO
 visibility.
 
-During the LTO link, all classes with public LTO vi

[PATCH] D127876: [clang] Don't emit type test/assume for virtual classes that should never participate in WPD

2022-06-16 Thread Arthur Eubanks 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 rGa70b39abffb4: [clang] Don't emit type test/assume for 
virtual classes that should never… (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127876

Files:
  clang/docs/LTOVisibility.rst
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/lto-visibility-inference.cpp

Index: clang/test/CodeGenCXX/lto-visibility-inference.cpp
===
--- clang/test/CodeGenCXX/lto-visibility-inference.cpp
+++ clang/test/CodeGenCXX/lto-visibility-inference.cpp
@@ -79,11 +79,11 @@
   // ITANIUM: type.test{{.*}}!"_ZTS2C4"
   // MS: type.test{{.*}}!"?AUC4@@"
   c4->f();
-  // ITANIUM: type.test{{.*}}!"_ZTS2C5"
-  // MS: type.test{{.*}}!"?AUC5@@"
+  // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C5"
+  // MS-NOT: type.test{{.*}}!"?AUC5@@"
   c5->f();
-  // ITANIUM: type.test{{.*}}!"_ZTS2C6"
-  // MS: type.test{{.*}}!"?AUC6@@"
+  // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C6"
+  // MS-NOT: type.test{{.*}}!"?AUC6@@"
   c6->f();
   // ITANIUM: type.test{{.*}}!"_ZTSSt2C7"
   // MS-STD: type.test{{.*}}!"?AUC7@std@@"
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -668,8 +668,8 @@
CGM.HasHiddenLTOVisibility(RD);
   bool ShouldEmitWPDInfo =
   CGM.getCodeGenOpts().WholeProgramVTables &&
-  // Don't insert type tests if we are forcing public std visibility.
-  !CGM.HasLTOVisibilityPublicStd(RD);
+  // Don't insert type tests if we are forcing public visibility.
+  !CGM.AlwaysHasLTOVisibilityPublic(RD);
   llvm::Value *VirtualFn = nullptr;
 
   {
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1382,10 +1382,10 @@
   /// optimization.
   bool HasHiddenLTOVisibility(const CXXRecordDecl *RD);
 
-  /// Returns whether the given record has public std LTO visibility
-  /// and therefore may not participate in (single-module) CFI and whole-program
-  /// vtable optimization.
-  bool HasLTOVisibilityPublicStd(const CXXRecordDecl *RD);
+  /// Returns whether the given record has public LTO visibility (regardless of
+  /// -lto-whole-program-visibility) and therefore may not participate in
+  /// (single-module) CFI and whole-program vtable optimization.
+  bool AlwaysHasLTOVisibilityPublic(const CXXRecordDecl *RD);
 
   /// Returns the vcall visibility of the given type. This is the scope in which
   /// a virtual function call could be made which ends up being dispatched to a
Index: clang/lib/CodeGen/CGVTables.cpp
===
--- clang/lib/CodeGen/CGVTables.cpp
+++ clang/lib/CodeGen/CGVTables.cpp
@@ -1175,7 +1175,10 @@
   DeferredVTables.clear();
 }
 
-bool CodeGenModule::HasLTOVisibilityPublicStd(const CXXRecordDecl *RD) {
+bool CodeGenModule::AlwaysHasLTOVisibilityPublic(const CXXRecordDecl *RD) {
+  if (RD->hasAttr() || RD->hasAttr())
+return true;
+
   if (!getCodeGenOpts().LTOVisibilityPublicStd)
 return false;
 
@@ -1200,9 +1203,6 @@
   if (!isExternallyVisible(LV.getLinkage()))
 return true;
 
-  if (RD->hasAttr() || RD->hasAttr())
-return false;
-
   if (getTriple().isOSBinFormatCOFF()) {
 if (RD->hasAttr() || RD->hasAttr())
   return false;
@@ -1211,7 +1211,7 @@
   return false;
   }
 
-  return !HasLTOVisibilityPublicStd(RD);
+  return !AlwaysHasLTOVisibilityPublic(RD);
 }
 
 llvm::GlobalObject::VCallVisibility CodeGenModule::GetVCallVisibilityLevel(
Index: clang/lib/CodeGen/CGClass.cpp
===
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -2695,9 +2695,9 @@
   if (SanOpts.has(SanitizerKind::CFIVCall))
 EmitVTablePtrCheckForCall(RD, VTable, CodeGenFunction::CFITCK_VCall, Loc);
   else if (CGM.getCodeGenOpts().WholeProgramVTables &&
-   // Don't insert type test assumes if we are forcing public std
+   // Don't insert type test assumes if we are forcing public
// visibility.
-   !CGM.HasLTOVisibilityPublicStd(RD)) {
+   !CGM.AlwaysHasLTOVisibilityPublic(RD)) {
 llvm::Metadata *MD =
 CGM.CreateMetadataIdentifierForType(QualType(RD->getTypeForDecl(), 0));
 llvm::Value *TypeId =
Index: clang/docs/LTOVisibility.rst
===
--- clang/docs/LTOVisibility.rst
+++ clang/docs/LTOVisibility.rst
@@ -35,13 +35,14 @@
 (e.g. classe

[PATCH] D127579: [clang][WIP] add option to keep types of ptr args for non-kernel functions in metadata

2022-06-16 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

If I'm understanding correctly, previously the LLVM pointee type was used to 
represent a frontend OpenCL type. An alternative to marking everything with 
`elementtype` or adding metadata, can something be done in the frontend to 
mangle the function name with OpenCL pointee types, then the backend translates 
that to the proper SPIRV types?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127579

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


[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

2022-06-09 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks requested changes to this revision.
aeubanks added a comment.
This revision now requires changes to proceed.

IIRC in the past there was a strong preference to not have the pass manager 
support this sort of thing
if you want to support this, there should be an RFC for how the optimization 
part of this will work as it may require invasive changes to the LLVM pass 
manager

(if this is purely a clang frontend thing then ignore me)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

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


[PATCH] D154093: [clang-format] Break long string literals in C#, etc.

2023-09-20 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D154093#4648931 , @eaeltsin wrote:

> This might have another issue with Verilog -
>
>   < import "AAA-BBB" foo bar baz
>   ---
>   > import {"AAA-",
>   > "BBB"} foo bar baz
>
> I wonder if Verilog allows breaking strings in `import`?

From the spec:
`dpi_spec_string ::= "DPI-C" | "DPI"`
Seems like it can't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154093

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


[PATCH] D154093: [clang-format] Break long string literals in C#, etc.

2023-09-20 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D154093#4648986 , @aeubanks wrote:

> In D154093#4648931 , @eaeltsin 
> wrote:
>
>> This might have another issue with Verilog -
>>
>>   < import "AAA-BBB" foo bar baz
>>   ---
>>   > import {"AAA-",
>>   > "BBB"} foo bar baz
>>
>> I wonder if Verilog allows breaking strings in `import`?
>
> From the spec:
> `dpi_spec_string ::= "DPI-C" | "DPI"`
> Seems like it can't.

Sent out https://github.com/llvm/llvm-project/pull/66951


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154093

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


[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

2023-07-24 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

the pipeline change and simplifycfg change should be split into two changes




Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:277
 
+static cl::opt AlwaysAllowSimplifyCFGSpeculation(
+"always-allow-simplify-cfg-speculation", cl::init(false), cl::Hidden,

can we just drop the flag and make this change?



Comment at: llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll:3
 ; RUN: opt < %s -S -passes=simplifycfg 
-simplifycfg-require-and-preserve-domtree=1 -bonus-inst-threshold=10 | 
FileCheck %s
+; RUN: opt < %s -S -passes='simplifycfg' 
-simplifycfg-require-and-preserve-domtree=1 -bonus-inst-threshold=10 | 
FileCheck %s --check-prefix=NOSPECULATE
 

these checks won't work for a `update_test_checks.py` test. should add tests 
like the one added in https://reviews.llvm.org/D153391



Comment at: llvm/test/Transforms/SimplifyCFG/pipeline-delay-speculation-pgo.ll:1
+;; Test that the pipelines delay simplify CFG speculation until after
+;; pgo annotation.

do you have a phase ordering test instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155997

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


[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

2023-07-24 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll:3
 ; RUN: opt < %s -S -passes=simplifycfg 
-simplifycfg-require-and-preserve-domtree=1 -bonus-inst-threshold=10 | 
FileCheck %s
+; RUN: opt < %s -S -passes='simplifycfg' 
-simplifycfg-require-and-preserve-domtree=1 -bonus-inst-threshold=10 | 
FileCheck %s --check-prefix=NOSPECULATE
 

tejohnson wrote:
> aeubanks wrote:
> > these checks won't work for a `update_test_checks.py` test. should add 
> > tests like the one added in https://reviews.llvm.org/D153391
> Can you clarify - are you saying remove the changes from this test and add a 
> new test for this change like 
> llvm/test/Transforms/SimplifyCFG/speculate-blocks.ll, or change the checks 
> here?
revert the changes here (`update_test_checks.py` tests shouldn't have manually 
added CHECK lines) and add new function(s) to 
`llvm/test/Transforms/SimplifyCFG/speculate-blocks.ll` that show the difference 
between `speculate-blocks` and `no-speculate-blocks`



Comment at: llvm/test/Transforms/SimplifyCFG/pipeline-delay-speculation-pgo.ll:1
+;; Test that the pipelines delay simplify CFG speculation until after
+;; pgo annotation.

tejohnson wrote:
> aeubanks wrote:
> > do you have a phase ordering test instead?
> Can you clarify what you are referring to?
`llvm/test/Transforms/PhaseOrdering` has IR tests that get run through an 
entire pipeline (e.g. `thinlto-pre-link`) to verify some property of the 
entire pipeline. ideally you can get some somewhat-reduced IR that exhibits the 
problem you were seeing before the change and show that it improves with the 
pipeline change. you'll need a profile as part of that test I suppose


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155997

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


[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

2023-08-17 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks created this revision.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This simply marks the global with the `unnamed_addr` IR attribute so it
be merged with other globals. This can break C++ pointer identity.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158223

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/unnamed-addr.c


Index: clang/test/CodeGen/unnamed-addr.c
===
--- /dev/null
+++ clang/test/CodeGen/unnamed-addr.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -O1 -emit-llvm -o - 
-disable-llvm-passes %s | FileCheck %s
+
+// CHECK: @i = unnamed_addr constant i32 8
+
+[[clang::unnamed_addr]] const int i = 8;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5254,6 +5254,9 @@
   GV->setConstant(true);
   }
 
+  if (D->hasAttr())
+GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+
   CharUnits AlignVal = getContext().getDeclAlign(D);
   // Check for alignment specifed in an 'omp allocate' directive.
   if (std::optional AlignValFromAllocate =
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -1408,6 +1408,24 @@
   }];
 }
 
+def UnnamedAddrDocs : Documentation {
+  let Category = DocCatField;
+  let Content = [{
+The ``clang::unnamed_addr`` attribute specifies that the address of a global is
+not significant. This allows global constants with the same contents to be
+merged. This can break global pointer identity, i.e. two different globals have
+the same address.
+
+Example usage:
+
+.. code-block:: c
+
+  // i and j may have the same address.
+  [[clang::unnamed_addr]] const int i = 42;
+  [[clang::unnamed_addr]] const int j = 42;
+  }];
+}
+
 def ObjCRequiresSuperDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1794,6 +1794,13 @@
   let SimpleHandler = 1;
 }
 
+def UnnamedAddr : InheritableAttr {
+  let Spellings = [Clang<"unnamed_addr">];
+  let Subjects = SubjectList<[GlobalVar], ErrorDiag>;
+  let Documentation = [UnnamedAddrDocs];
+  let SimpleHandler = 1;
+}
+
 def ReturnsTwice : InheritableAttr {
   let Spellings = [GCC<"returns_twice">];
   let Subjects = SubjectList<[Function]>;


Index: clang/test/CodeGen/unnamed-addr.c
===
--- /dev/null
+++ clang/test/CodeGen/unnamed-addr.c
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -O1 -emit-llvm -o - -disable-llvm-passes %s | FileCheck %s
+
+// CHECK: @i = unnamed_addr constant i32 8
+
+[[clang::unnamed_addr]] const int i = 8;
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5254,6 +5254,9 @@
   GV->setConstant(true);
   }
 
+  if (D->hasAttr())
+GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+
   CharUnits AlignVal = getContext().getDeclAlign(D);
   // Check for alignment specifed in an 'omp allocate' directive.
   if (std::optional AlignValFromAllocate =
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -1408,6 +1408,24 @@
   }];
 }
 
+def UnnamedAddrDocs : Documentation {
+  let Category = DocCatField;
+  let Content = [{
+The ``clang::unnamed_addr`` attribute specifies that the address of a global is
+not significant. This allows global constants with the same contents to be
+merged. This can break global pointer identity, i.e. two different globals have
+the same address.
+
+Example usage:
+
+.. code-block:: c
+
+  // i and j may have the same address.
+  [[clang::unnamed_addr]] const int i = 42;
+  [[clang::unnamed_addr]] const int j = 42;
+  }];
+}
+
 def ObjCRequiresSuperDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1794,6 +1794,13 @@
   let SimpleHandler = 1;
 }
 
+def UnnamedAddr : InheritableAttr {
+  let Spellings = [Clang<"unnamed_addr">];
+  let Subjects = SubjectList<

[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

2023-08-17 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

unsure if this should have an RFC or not

will add release notes if this is ok to go ahead with


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158223

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


[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

2023-08-17 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

and naming can be bikeshed. `no_addrsig` follows 
https://llvm.org/docs/Extensions.html#sht-llvm-addrsig-section-address-significance-table,
 `unnamed_addr` follows the IR attribute


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158223

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


[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

2023-08-17 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

merging doesn't have to be done at link time, the optimizer can also do it. so 
I'd rule out any names with `icf` since that's specifically the name of a 
linker feature

`no_unique_address` popped into my mind but that's already taken :). that's 
exactly what this is though. maybe `no_unique_identity`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158223

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


[PATCH] D156799: Update generic scheduling to use A510 scheduling model

2023-08-21 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll:1
-; RUN: split-file %s %t
 ; RUN: cat %t/main.ll %t/a.ll > %t/a2.ll
 ; RUN: cat %t/main.ll %t/b.ll > %t/b2.ll

this test is broken, without the `split-file` the other files won't exist in a 
clean build, please fix or revert if it takes time to fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156799

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


[PATCH] D157879: [clang] Report missing designated initializers in C++

2023-08-21 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

is there any way to suppress this for a specific field? (I believe) I'm seeing 
user code where a field is intentionally not being initialized


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157879

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


[PATCH] D157879: [clang] Report missing designated initializers in C++

2023-08-21 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

ah I thought this was in `-Wall` but it's not

the struct is

  struct Foo {
const void* buffer;
uint32_t capacity;
uint32_t reserved;
  };

where `reserved` isn't explicitly initialized. that seems like reasonable code, 
but I suppose we can just explicitly initialize `reserved` here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157879

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


[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

2023-08-21 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: compiler-rt/lib/profile/InstrProfilingMerge.c:146
+if (!(__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE)) {
+  PROF_ERR("%s\n", "Missing profile data section.");
+  return 1;

we're running into this error message even though we don't have debug info 
correlation turned on, is that expected in some configurations? I'm still 
trying to understand how we could end up in this code path without debug info 
correlation

https://crbug.com/1473935


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

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


[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

2023-08-21 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: compiler-rt/lib/profile/InstrProfilingMerge.c:146
+if (!(__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE)) {
+  PROF_ERR("%s\n", "Missing profile data section.");
+  return 1;

aeubanks wrote:
> we're running into this error message even though we don't have debug info 
> correlation turned on, is that expected in some configurations? I'm still 
> trying to understand how we could end up in this code path without debug info 
> correlation
> 
> https://crbug.com/1473935
also it seems like this code path isn't tested?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

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


[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

2023-08-22 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:1416-1417
+not significant. This allows global constants with the same contents to be
+merged. This can break global pointer identity, i.e. two different globals have
+the same address.
+

aaron.ballman wrote:
> What happens for tentative definitions where the value isn't known? e.g.,
> ```
> [[clang::unnamed_addr]] int i1, i2;
> ```
> 
> What happens if the types are similar but not the same?
> ```
> [[clang::unnamed_addr]] signed int i1 = 32;
> [[clang::unnamed_addr]] unsigned int i2 = 32;
> ```
> 
> Should we diagnose taking the address of such an attributed variable so users 
> have some hope of spotting the non-conforming situations?
> 
> Does this attribute have impacts across translation unit boundaries (perhaps 
> only when doing LTO) or only within a single TU?
> 
> What does this attribute do in C++ in the presence of constructors and 
> destructors? e.g.,
> ```
> struct S {
>   S();
>   ~S();
> };
> 
> [[clang::unnamed_addr]] S s1, s2; // Are these merged and there's only one 
> ctor/dtor call?
> ```
globals are only mergeable if they're known to be constant and have the same 
value/size. this can be done at compile time only if the optimizer can see the 
constant values, or at link time

so nothing would happen in any of the cases you've given.

but yeah that does imply that we should warn when the attribute is used on non 
const, non-POD globals. I'll update this patch to do that

as mentioned in the description, we actually do want to take the address of 
these globals for table-driven parsing, but we don't care about identity 
equality


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158223

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


[PATCH] D157879: [clang] Report missing designated initializers in C++

2023-08-22 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D157879#4606531 , @aaron.ballman 
wrote:

> In D157879#4604288 , @phosek wrote:
>
>> Note that there's an ongoing discussion on the GCC bug 
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96868 whether 
>> `-Wmissing-field-initializers` should apply to both C and C++ or just C.
>
> Thank you for posting this! I'm with @jwakely on this: 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96868#c3 -- the warning is for 
> people who want to be told they've not explicitly specified all of the 
> initializers, it is not for telling you "this hasn't been initialized". I 
> think the desire makes as much sense in C++ as it does in C, but this is more 
> of a style warning than a correctness warning.
>
> In D157879#4604471 , @ianloic wrote:
>
>> In D157879#4604233 , @aeubanks 
>> wrote:
>>
>>> ah I thought this was in `-Wall` but it's not
>>>
>>> the struct is
>>>
>>>   struct Foo {
>>> const void* buffer;
>>> uint32_t capacity;
>>> uint32_t reserved;
>>>   };
>>>
>>> where `reserved` isn't explicitly initialized. that seems like reasonable 
>>> code, but I suppose we can just explicitly initialize `reserved` here
>>
>> FWIW, if this is the specific piece of code that led me to this 
>> conversation, that declaration is in a header which at least in theory is 
>> supposed to remain compatible with C. It's inside `extern "C" { ... }` even. 
>> We can't add explicit initialize `reserved` and keep the definition C 
>> comaptible.
>
> Ouch, yeah, that does make it a challenge. But why is 
> `-Wmissing-field-initializers` enabled in the first place for the code? It 
> seems like there are fields you *don't* want explicit initialization for, so 
> perhaps the answer is to disable the diagnostic for those types?

It's an unfortunate configuration. We saw this in Chrome where some projects 
were manually setting `-Wmissing-field-initializers`, but when building for 
Fuchsia, some Fuchsia system headers had this code. We can just suppress this 
for all Fuchsia builds, I think that's fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157879

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


[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

2023-08-22 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: compiler-rt/lib/profile/InstrProfilingMerge.c:146
+if (!(__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE)) {
+  PROF_ERR("%s\n", "Missing profile data section.");
+  return 1;

ellis wrote:
> aeubanks wrote:
> > aeubanks wrote:
> > > we're running into this error message even though we don't have debug 
> > > info correlation turned on, is that expected in some configurations? I'm 
> > > still trying to understand how we could end up in this code path without 
> > > debug info correlation
> > > 
> > > https://crbug.com/1473935
> > also it seems like this code path isn't tested?
> It's very strange that you are hitting this code path because I didn't think 
> it was possible. Were you ever able to reproduce locally?
no, it's only showed up on some bots (both mac and linux), I'm still trying to 
understand what's going on


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

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


[PATCH] D158223: [clang] Add clang::unnamed_addr attribute that marks globals' address as not significant

2023-08-22 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:1416-1417
+not significant. This allows global constants with the same contents to be
+merged. This can break global pointer identity, i.e. two different globals have
+the same address.
+

aaron.ballman wrote:
> aeubanks wrote:
> > aaron.ballman wrote:
> > > What happens for tentative definitions where the value isn't known? e.g.,
> > > ```
> > > [[clang::unnamed_addr]] int i1, i2;
> > > ```
> > > 
> > > What happens if the types are similar but not the same?
> > > ```
> > > [[clang::unnamed_addr]] signed int i1 = 32;
> > > [[clang::unnamed_addr]] unsigned int i2 = 32;
> > > ```
> > > 
> > > Should we diagnose taking the address of such an attributed variable so 
> > > users have some hope of spotting the non-conforming situations?
> > > 
> > > Does this attribute have impacts across translation unit boundaries 
> > > (perhaps only when doing LTO) or only within a single TU?
> > > 
> > > What does this attribute do in C++ in the presence of constructors and 
> > > destructors? e.g.,
> > > ```
> > > struct S {
> > >   S();
> > >   ~S();
> > > };
> > > 
> > > [[clang::unnamed_addr]] S s1, s2; // Are these merged and there's only 
> > > one ctor/dtor call?
> > > ```
> > globals are only mergeable if they're known to be constant and have the 
> > same value/size. this can be done at compile time only if the optimizer can 
> > see the constant values, or at link time
> > 
> > so nothing would happen in any of the cases you've given.
> > 
> > but yeah that does imply that we should warn when the attribute is used on 
> > non const, non-POD globals. I'll update this patch to do that
> > 
> > as mentioned in the description, we actually do want to take the address of 
> > these globals for table-driven parsing, but we don't care about identity 
> > equality
> > globals are only mergeable if they're known to be constant and have the 
> > same value/size. this can be done at compile time only if the optimizer can 
> > see the constant values, or at link time
> >
> > so nothing would happen in any of the cases you've given.
> 
> A that's good to know. So I assume we *will* merge these?
> 
> ```
> struct S {
>   int i, j;
>   float f;
> };
> 
> [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> [[clang::unnamed_addr]] const S s3 = s2;
> ```
> 
> > but yeah that does imply that we should warn when the attribute is used on 
> > non const, non-POD globals. I'll update this patch to do that
> 
> Thank you, I think that will be more user-friendly
> 
> > as mentioned in the description, we actually do want to take the address of 
> > these globals for table-driven parsing, but we don't care about identity 
> > equality
> 
> Yeah, I still wonder if we want to diagnose just the same -- if the address 
> is never taken, there's not really a way to notice the optimization, but if 
> the address is taken, you basically get UB (and I think we should explicitly 
> document it as such). Given how easy it is to accidentally take the address 
> of something (like via a reference in C++), I think we should warn by 
> default, but still have a warning group for folks who want to live life 
> dangerously.
> > globals are only mergeable if they're known to be constant and have the 
> > same value/size. this can be done at compile time only if the optimizer can 
> > see the constant values, or at link time
> >
> > so nothing would happen in any of the cases you've given.
> 
> A that's good to know. So I assume we *will* merge these?
> 
> ```
> struct S {
>   int i, j;
>   float f;
> };
> 
> [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> [[clang::unnamed_addr]] const S s3 = s2;
> ```
yeah those are merged even just by clang

```
struct S {
  int i, j;
  float f;
};

[[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
[[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
[[clang::unnamed_addr]] const S s3 = s2;

const void * f1() {
  return &s1;
}

const void * f2() {
  return &s2;
}

const void * f3() {
  return &s3;
}

$ ./build/rel/bin/clang++ -S -emit-llvm -o - -O2 /tmp/a.cc
```
> 
> > but yeah that does imply that we should warn when the attribute is used on 
> > non const, non-POD globals. I'll update this patch to do that
> 
> Thank you, I think that will be more user-friendly
> 
> > as mentioned in the description, we actually do want to take the address of 
> > these globals for table-driven parsing, but we don't care about identity 
> > equality
> 
> Yeah, I still wonder if we want to diagnose just the same -- if the address 
> is never taken, there's not really a way to notice the optimization, but if 
> the address is taken, you basically get UB (and I think we should explicitly 
> document it as such). Given how easy it is to accidentally take the address 
> of something (like via a re

[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

2023-08-22 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: compiler-rt/lib/profile/InstrProfilingMerge.c:146
+if (!(__llvm_profile_get_version() & VARIANT_MASK_DBG_CORRELATE)) {
+  PROF_ERR("%s\n", "Missing profile data section.");
+  return 1;

aeubanks wrote:
> ellis wrote:
> > aeubanks wrote:
> > > aeubanks wrote:
> > > > we're running into this error message even though we don't have debug 
> > > > info correlation turned on, is that expected in some configurations? 
> > > > I'm still trying to understand how we could end up in this code path 
> > > > without debug info correlation
> > > > 
> > > > https://crbug.com/1473935
> > > also it seems like this code path isn't tested?
> > It's very strange that you are hitting this code path because I didn't 
> > think it was possible. Were you ever able to reproduce locally?
> no, it's only showed up on some bots (both mac and linux), I'm still trying 
> to understand what's going on
managed to repro having no `DataSize` (`NumData` now).

```
$ cat /tmp/a.cc
int main() {}
$ cat /tmp/funlist
[clang]
default:skip
$ clang++ -O1 -fprofile-instr-generate -fcoverage-mapping -o /tmp/a 
-fuse-ld=lld /tmp/z.cc -fprofile-list=/tmp/funlist
$ export LLVM_PROFILE_FILE='a%m.profraw'
$ /tmp/a
$ /tmp/a
LLVM Profile Error: Missing profile data section.
LLVM Profile Error: Invalid profile data to merge
LLVM Profile Error: Profile Merging of file a18405209413954990729_0.profraw 
failed: Success
LLVM Profile Error: Failed to write file "a18405209413954990729_0.profraw": 
Success
```

basically if nothing is instrumented we'll hit this

will revert


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

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


[PATCH] D157632: [Profile] Allow online merging with debug info correlation.

2023-08-22 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks reopened this revision.
aeubanks added a comment.
This revision is now accepted and ready to land.

reverted in 9b6b6bbc9127d604881cdc9dcea0e7c74ef821fd 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157632

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


[PATCH] D137149: Use PassGate from LLVMContext if any otherwise global one

2023-10-06 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

we don't currently support reusing a pipeline so I'm surprised that you're able 
to share/reuse pipelines without running into any issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137149

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


[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

2023-10-06 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

This is finding lots of real issues in code, which is awesome, but could I 
request that this be put under a separate warning flag so we can toggle off 
just the new functionality and turn it on as we clean our codebase?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153131

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


[PATCH] D137149: Use PassGate from LLVMContext if any otherwise global one

2023-10-09 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

In D137149#4653381 , @vchuravy wrote:

> In D137149#4653308 , @aeubanks 
> wrote:
>
>> we don't currently support reusing a pipeline so I'm surprised that you're 
>> able to share/reuse pipelines without running into any issues
>
> In addition to @pchintalapudi's comment. Reuse of pipelines is something that 
> we (JuliaLang) had come to expect from old PM. This is why opt had a 
> `-run-twice` option to help flush
> out bugs that arose out of the idea that passes would only be used once. 
> @loladiro might remember those discussions.
>
> So when we ported Julia to NewPM I didn't think twice and @pchintalapudi 
> implemented our NewPM usage such that only the AnalysisManager
> would be created fresh.

Passes can store state that might break between runs (especially module passes 
since they typically only run once). I'm not saying it's impossible to reuse a 
pipeline, just that it's not tested, e.g. `-run-twice` isn't hooked up to the 
new PM. There was a discussion about this before somewhere, can't remember 
where. We'd need a lot more testing before we can claim to support reusing 
pipelines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137149

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


[PATCH] D153131: [clang analysis][thread-safety] Handle return-by-reference...

2023-10-09 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

If people are passing `-Wthread-safety-reference`, there was clearly some value 
in the previous checks and it would be unfortunate to turn them off while 
fixing the codebase.
I'm not super familiar with flag families and if what I'm proposing is easily 
doable, but I think ideally we would keep this new functionality turned on by 
default under `-Wthread-safety-reference` and make just this new functionality 
toggleable under a subflag `-W[no-]thread-safety-reference-return`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153131

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


[PATCH] D149800: [WIP][PGO] Add ability to mark cold functions as optsize/minsize/optnone

2023-10-13 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks abandoned this revision.
aeubanks added a comment.

reworked into https://github.com/llvm/llvm-project/pull/69030


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149800

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


[PATCH] D151479: [clang] Use IgnoreParensSingleStep in more places

2023-10-16 Thread Arthur Eubanks 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 rGe32cde6f41cd: [clang] Use IgnoreParensSingleStep in more 
places (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151479

Files:
  clang/lib/Sema/SemaInit.cpp


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
+#include "clang/AST/IgnoreExpr.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/SourceManager.h"
@@ -170,22 +171,9 @@
   while (true) {
 E->setType(Ty);
 E->setValueKind(VK_PRValue);
-if (isa(E) || isa(E)) {
-  break;
-} else if (ParenExpr *PE = dyn_cast(E)) {
-  E = PE->getSubExpr();
-} else if (UnaryOperator *UO = dyn_cast(E)) {
-  assert(UO->getOpcode() == UO_Extension);
-  E = UO->getSubExpr();
-} else if (GenericSelectionExpr *GSE = dyn_cast(E)) {
-  E = GSE->getResultExpr();
-} else if (ChooseExpr *CE = dyn_cast(E)) {
-  E = CE->getChosenSubExpr();
-} else if (PredefinedExpr *PE = dyn_cast(E)) {
-  E = PE->getFunctionName();
-} else {
-  llvm_unreachable("unexpected expr in string literal init");
-}
+if (isa(E) || isa(E))
+  break;
+E = IgnoreParensSingleStep(E);
   }
 }
 
@@ -194,20 +182,9 @@
 static void updateGNUCompoundLiteralRValue(Expr *E) {
   while (true) {
 E->setValueKind(VK_PRValue);
-if (isa(E)) {
-  break;
-} else if (ParenExpr *PE = dyn_cast(E)) {
-  E = PE->getSubExpr();
-} else if (UnaryOperator *UO = dyn_cast(E)) {
-  assert(UO->getOpcode() == UO_Extension);
-  E = UO->getSubExpr();
-} else if (GenericSelectionExpr *GSE = dyn_cast(E)) {
-  E = GSE->getResultExpr();
-} else if (ChooseExpr *CE = dyn_cast(E)) {
-  E = CE->getChosenSubExpr();
-} else {
-  llvm_unreachable("unexpected expr in array compound literal init");
-}
+if (isa(E))
+  break;
+E = IgnoreParensSingleStep(E);
   }
 }
 


Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
+#include "clang/AST/IgnoreExpr.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/SourceManager.h"
@@ -170,22 +171,9 @@
   while (true) {
 E->setType(Ty);
 E->setValueKind(VK_PRValue);
-if (isa(E) || isa(E)) {
-  break;
-} else if (ParenExpr *PE = dyn_cast(E)) {
-  E = PE->getSubExpr();
-} else if (UnaryOperator *UO = dyn_cast(E)) {
-  assert(UO->getOpcode() == UO_Extension);
-  E = UO->getSubExpr();
-} else if (GenericSelectionExpr *GSE = dyn_cast(E)) {
-  E = GSE->getResultExpr();
-} else if (ChooseExpr *CE = dyn_cast(E)) {
-  E = CE->getChosenSubExpr();
-} else if (PredefinedExpr *PE = dyn_cast(E)) {
-  E = PE->getFunctionName();
-} else {
-  llvm_unreachable("unexpected expr in string literal init");
-}
+if (isa(E) || isa(E))
+  break;
+E = IgnoreParensSingleStep(E);
   }
 }
 
@@ -194,20 +182,9 @@
 static void updateGNUCompoundLiteralRValue(Expr *E) {
   while (true) {
 E->setValueKind(VK_PRValue);
-if (isa(E)) {
-  break;
-} else if (ParenExpr *PE = dyn_cast(E)) {
-  E = PE->getSubExpr();
-} else if (UnaryOperator *UO = dyn_cast(E)) {
-  assert(UO->getOpcode() == UO_Extension);
-  E = UO->getSubExpr();
-} else if (GenericSelectionExpr *GSE = dyn_cast(E)) {
-  E = GSE->getResultExpr();
-} else if (ChooseExpr *CE = dyn_cast(E)) {
-  E = CE->getChosenSubExpr();
-} else {
-  llvm_unreachable("unexpected expr in array compound literal init");
-}
+if (isa(E))
+  break;
+E = IgnoreParensSingleStep(E);
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

2023-07-28 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: llvm/test/Transforms/PhaseOrdering/simplifycfg-speculate-blocks.ll:1
+;; Check that SimplifyCFG does not attempt speculation until after PGO is
+;; annotated in the IR, and then does not perform it when unprofitable.

hmm typically these phase ordering tests use 
`llvm/utils/update_test_checks.py`, but that doesn't support the llvm-profdata 
RUN line. I think a non-update_test_checks test is probably fine for this, 
@nikic does that make sense?





Comment at: 
llvm/test/Transforms/PhaseOrdering/simplifycfg-speculate-blocks.ll:22
+; RUN: opt < %s -passes='thinlto-pre-link' 
-pgo-kind=pgo-sample-use-pipeline 
-profile-file=%S/Inputs/simplifycfg-speculate-blocks.sampleprof -S | FileCheck 
%s --check-prefixes=NO,SAMPLE
+; RUN: opt < %s -passes='thinlto-pre-link' 
-pgo-kind=pgo-sample-use-pipeline 
-profile-file=%S/Inputs/simplifycfg-speculate-blocks.sampleprof -S | FileCheck 
%s --check-prefixes=NO,SAMPLE
+

`lto-pre-link`



Comment at: 
llvm/test/Transforms/PhaseOrdering/simplifycfg-speculate-blocks.ll:83
+!4 = !{i32 1, !"wchar_size", i32 4}
+!5 = !{i32 8, !"PIC Level", i32 2}
+!6 = !{i32 7, !"PIE Level", i32 2}

the debug info is necessary for sample profile to work I presume?

I think some of this can still be simplified, like `PIC Level`, etc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155997

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


[PATCH] D155997: [Phase Ordering] Don't speculate in SimplifyCFG before PGO annotation

2023-08-08 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

can we try not gating this on PGO as suggested? minimizing differences between 
pipelines is nice, and as mentioned it'll help with other cases




Comment at: llvm/test/Transforms/PhaseOrdering/simplifycfg-speculate-blocks.ll:1
+;; Check that SimplifyCFG does not attempt speculation until after PGO is
+;; annotated in the IR, and then does not perform it when unprofitable.

nikic wrote:
> aeubanks wrote:
> > hmm typically these phase ordering tests use 
> > `llvm/utils/update_test_checks.py`, but that doesn't support the 
> > llvm-profdata RUN line. I think a non-update_test_checks test is probably 
> > fine for this, @nikic does that make sense?
> > 
> > 
> I thought UTC supports unrecognized commands (by just executing them)... If 
> not, a non-UTC test is fine.
`update_test_checks.py` doesn't handle `%t` (maybe that's an improvement we 
could make to the script)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155997

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


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

2023-08-09 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks accepted this revision.
aeubanks added a comment.
This revision is now accepted and ready to land.

seems fine if nobody else objects


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] D158474: [clang][ExtractAPI] Fix bool spelling coming from the macro definition.

2023-09-07 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: clang/test/ExtractAPI/bool.c:14
+//--- input.h
+#include 
+bool Foo;

aeubanks wrote:
> clang tests should not be including system headers since that makes the tests 
> non-hermetic (this test fails in our internal builds that provide more 
> isolated test environments), could you make this not include ?
ignore my previous comment, stdbool.h is provided by clang, sorry for the 
trouble


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158474

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


[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

2023-09-11 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1102
+outs() << "\n";
+return;
+  }

I wouldn't return here, doesn't seem right that we'll skip running the opt 
pipeline but continue with compilation. we should either bail out entirely  of 
producing any output files (which would probably require code changes 
elsewhere), or run everything as normal, not do something weird where we don't 
run the optimization pipeline but still output files



Comment at: clang/test/CodeGen/print-pipeline-passes.c:8
+// CHECK: always-inline
+// CHECK-SAME: BitcodeWriterPass
+void Foo(void) {}

I wouldn't test BitcodeWriterPass, we may have a proper textual name for it at 
some point


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127221

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


[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

2023-09-11 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks accepted this revision.
aeubanks added a comment.

lg with one test comment




Comment at: clang/lib/CodeGen/BackendUtil.cpp:1102
+outs() << "\n";
+return;
+  }

jcranmer-intel wrote:
> aeubanks wrote:
> > I wouldn't return here, doesn't seem right that we'll skip running the opt 
> > pipeline but continue with compilation. we should either bail out entirely  
> > of producing any output files (which would probably require code changes 
> > elsewhere), or run everything as normal, not do something weird where we 
> > don't run the optimization pipeline but still output files
> This is basically doing the same thing that `opt -print-pipeline-passes` is 
> doing: 
> https://github.com/llvm/llvm-project/blob/d1b418f55263ec48d14f220ad020d55f126cfcab/llvm/tools/opt/NewPMDriver.cpp#L500-L524.
> 
> In the case of emitting LLVM IR or bitcode, this logic is sufficient to not 
> emit any output. In the case of .s or .o files, it looks like I have to also 
> bail out of running `RunCodegenPipeline`
if skipping `RunCodegenPipeline` doesn't produce any output file, sgtm



Comment at: clang/test/CodeGen/print-pipeline-passes.c:8
+// CHECK: always-inline
+// CHECK-SAME: verify
+void Foo(void) {}

I believe this will fail in non-assert builds


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127221

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


[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

2023-09-11 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: clang/test/CodeGen/print-pipeline-passes.c:8
+// CHECK: always-inline
+// CHECK-SAME: verify
+void Foo(void) {}

aeubanks wrote:
> I believe this will fail in non-assert builds
since we don't run the verifier in non-assert builds


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127221

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


[PATCH] D153924: [OpenMP] Allow exceptions in target regions when offloading to GPUs

2023-09-11 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: clang/test/OpenMP/amdgpu_throw_trap.cpp:4
+// RUN: %clang_cc1 -fopenmp -triple amdgcn-amd-amdhsa 
-fopenmp-is-target-device %s -emit-llvm -S -Wno-openmp-target-exception -o - | 
FileCheck -check-prefix=DEVICE %s
+// RUN: %clang_cc1 -fopenmp -triple x86_64-pc-linux-gnu 
-fopenmp-is-target-device -fcxx-exceptions %s -emit-llvm -S 
-Wno-openmp-target-exception -o - | FileCheck -check-prefix=HOST %s
+// DEVICE: s_trap

thakis wrote:
> This test fails if X86 isn't in `LLVM_TARGETS_TO_BUILD` and the host system 
> is some non-x86 system (e.g. arm64).
> 
> (This is the only test in check-clang that fails then.)
> 
> Should this test grow a `REQUIRES: x86-registered-target`? Should it use 
> `%itanium_abi_triple` instead of `x86_64-pc-linux-gnu`? (It seems to pass 
> when replacing `x86_64-pc-linux-gnu` with `%itanium_abi_triple` on my arm 
> mac.)
added x86-registered-target in 238a1ef44f4f2361205e538b3cb7ebc5ec70894d


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153924

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


[PATCH] D153924: [OpenMP] Allow exceptions in target regions when offloading to GPUs

2023-09-11 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: clang/test/OpenMP/amdgpu_throw_trap.cpp:4
+// RUN: %clang_cc1 -fopenmp -triple amdgcn-amd-amdhsa 
-fopenmp-is-target-device %s -emit-llvm -S -Wno-openmp-target-exception -o - | 
FileCheck -check-prefix=DEVICE %s
+// RUN: %clang_cc1 -fopenmp -triple x86_64-pc-linux-gnu 
-fopenmp-is-target-device -fcxx-exceptions %s -emit-llvm -S 
-Wno-openmp-target-exception -o - | FileCheck -check-prefix=HOST %s
+// DEVICE: s_trap

thakis wrote:
> aeubanks wrote:
> > thakis wrote:
> > > This test fails if X86 isn't in `LLVM_TARGETS_TO_BUILD` and the host 
> > > system is some non-x86 system (e.g. arm64).
> > > 
> > > (This is the only test in check-clang that fails then.)
> > > 
> > > Should this test grow a `REQUIRES: x86-registered-target`? Should it use 
> > > `%itanium_abi_triple` instead of `x86_64-pc-linux-gnu`? (It seems to pass 
> > > when replacing `x86_64-pc-linux-gnu` with `%itanium_abi_triple` on my arm 
> > > mac.)
> > added x86-registered-target in 238a1ef44f4f2361205e538b3cb7ebc5ec70894d
> Is that better than `%itanium_abi_triple`?
I was worried about LLVM failing if the calculated `%itanium_abi_triple` wasn't 
supported in that build of LLVM, but TIL that clang/LLVM can handle triples it 
doesn't recognize all the way until the codegen phase. But IIUC optimizations 
can change depending on whether or not LLVM recognizes the triple so it's still 
a little inconsistent.

so yeah `%itanium_abi_triple` would probably work, but it seems susceptible to 
configuration differences


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153924

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


[PATCH] D127221: [Clang] Enable -print-pipeline-passes in clang.

2023-09-12 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: clang/test/CodeGen/print-pipeline-passes.c:8
+// CHECK: always-inline
+// CHECK-SAME: verify
+void Foo(void) {}

jcranmer-intel wrote:
> aeubanks wrote:
> > aeubanks wrote:
> > > I believe this will fail in non-assert builds
> > since we don't run the verifier in non-assert builds
> Apparently verify is added by clang unless `-disable-llvm-verifier` is 
> present on the command line, but I've switched it to annotation-remarks 
> nevertheless.
`-disable-llvm-verifier` is added by clang in non-assert builds


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127221

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


[PATCH] D130531: [IR] Use Min behavior for module flag "PIC Level"

2023-09-12 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added a comment.

I'm not understanding why this doesn't also apply to "PIE Level", doesn't it 
also follow the same reasoning? pic -> PIC is the same as pie -> PIE

e.g. if you merge a small PIC and large PIC file, the resulting file would only 
be guaranteed to work with a "large PIC executable" (unsure what the right term 
is) and not a "small PIC executable", so if we say it's a large PIC file, 
that's wrong since it wouldn't link into a "large PIC executable", so we have 
to conservatively say it's a small PIC file.
and s/PIC/PIE for the same argument


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130531

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


[PATCH] D154253: [clang] detect integer overflow through temporary values

2023-06-30 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:15215
   Exprs.append(Construct->arg_begin(), Construct->arg_end());
+else if (auto Temporary = dyn_cast(E))
+  Exprs.push_back(Temporary->getSubExpr());

nit: `clang::` isn't necessary


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154253

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


<    1   2   3   4   5   6   7   8   >