[PATCH] D139801: [clang-format] Adds 'friend' to QualifierOrder.

2022-12-12 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

I know I have situations where I like `inline` and `static` to be in a certain 
order to avoid them getting constantly swapped around, they are never going to 
be after the type but I still want them the same way, I'd be ok with allow 
`friend` for the same reason.

I'm just pleased people find this feature useful, despite the original concern 
raised and the hairiness of the implementation. I also appreciate the 
concessions made by others to allow clang-format to alter code as this has 
opened the doors in my view to 1 or 2 other great code modification changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139801

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

[PATCH] D139834: [clang-format] AllowShortCompoundRequirementOnASingleLine

2022-12-12 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/Format.cpp:809
Style.AllowShortCaseLabelsOnASingleLine);
+IO.mapOptional("AllowShortCompoundRequirementOnASingleLine",
+   Style.AllowShortCompoundRequirementOnASingleLine);

haven't we use "Requires" in other options? What is the definition of Compound?

I might be tempted for this to be AllShortRequiresOnASingleLine but then it be 
an enum with the following options

```
Leave
Never
Always
Compound
```



Comment at: clang/lib/Format/Format.cpp:1265
   LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
+  LLVMStyle.AllowShortCompoundRequirementOnASingleLine = true;
   LLVMStyle.AllowShortEnumsOnASingleLine = true;

why would the default be true, is that what happens today?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139834

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

[PATCH] D139915: [Clang][LoongArch] Add intrinsic for asrtle, asrtgt, lddir, ldpte and cpucfg

2022-12-12 Thread Gong LingQin via Phabricator via cfe-commits 
gonglingqin updated this revision to Diff 482372.
gonglingqin added a comment.

Address @xen0n's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139915

Files:
  clang/include/clang/Basic/BuiltinsLoongArch.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/larchintrin.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/LoongArch/intrinsic-la32-error.c
  clang/test/CodeGen/LoongArch/intrinsic-la32.c
  clang/test/CodeGen/LoongArch/intrinsic-la64-error.c
  clang/test/CodeGen/LoongArch/intrinsic-la64.c
  llvm/include/llvm/IR/IntrinsicsLoongArch.td
  llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
  llvm/lib/Target/LoongArch/LoongArchISelLowering.h
  llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
  llvm/test/CodeGen/LoongArch/intrinsic-la32-error.ll
  llvm/test/CodeGen/LoongArch/intrinsic-la64.ll
  llvm/test/CodeGen/LoongArch/intrinsic.ll

Index: llvm/test/CodeGen/LoongArch/intrinsic.ll
===
--- llvm/test/CodeGen/LoongArch/intrinsic.ll
+++ llvm/test/CodeGen/LoongArch/intrinsic.ll
@@ -15,6 +15,7 @@
 declare void @llvm.loongarch.iocsrwr.b(i32, i32)
 declare void @llvm.loongarch.iocsrwr.h(i32, i32)
 declare void @llvm.loongarch.iocsrwr.w(i32, i32)
+declare i32 @llvm.loongarch.cpucfg(i32)
 
 define void @foo() nounwind {
 ; CHECK-LABEL: foo:
@@ -145,3 +146,13 @@
   tail call void @llvm.loongarch.iocsrwr.w(i32 %a, i32 %b)
   ret void
 }
+
+define i32 @cpucfg(i32 %a) {
+; CHECK-LABEL: cpucfg:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:cpucfg $a0, $a0
+; CHECK-NEXT:ret
+entry:
+  %0 = tail call i32 @llvm.loongarch.cpucfg(i32 %a)
+  ret i32 %0
+}
Index: llvm/test/CodeGen/LoongArch/intrinsic-la64.ll
===
--- llvm/test/CodeGen/LoongArch/intrinsic-la64.ll
+++ llvm/test/CodeGen/LoongArch/intrinsic-la64.ll
@@ -14,6 +14,10 @@
 declare i64 @llvm.loongarch.csrxchg.d(i64, i64, i32 immarg)
 declare i64 @llvm.loongarch.iocsrrd.d(i32)
 declare void @llvm.loongarch.iocsrwr.d(i64, i32)
+declare void @llvm.loongarch.asrtle.d(i64, i64)
+declare void @llvm.loongarch.asrtgt.d(i64, i64)
+declare i64 @llvm.loongarch.lddir.d(i64, i64)
+declare void @llvm.loongarch.ldpte.d(i64, i64)
 
 define i32 @crc_w_b_w(i32 %a, i32 %b) nounwind {
 ; CHECK-LABEL: crc_w_b_w:
@@ -136,3 +140,43 @@
   tail call void @llvm.loongarch.iocsrwr.d(i64 %a, i32 %b)
   ret void
 }
+
+define void @asrtle_d(i64 %a, i64 %b) {
+; CHECK-LABEL: asrtle_d:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:asrtle.d $a0, $a1
+; CHECK-NEXT:ret
+entry:
+  tail call void @llvm.loongarch.asrtle.d(i64 %a, i64 %b)
+  ret void
+}
+
+define void @asrtgt_d(i64 %a, i64 %b) {
+; CHECK-LABEL: asrtgt_d:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:asrtgt.d $a0, $a1
+; CHECK-NEXT:ret
+entry:
+  tail call void @llvm.loongarch.asrtgt.d(i64 %a, i64 %b)
+  ret void
+}
+
+define i64 @lddir_d(i64 %a) {
+; CHECK-LABEL: lddir_d:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:lddir $a0, $a0, 1
+; CHECK-NEXT:ret
+entry:
+  %0 = tail call i64 @llvm.loongarch.lddir.d(i64 %a, i64 1)
+  ret i64 %0
+}
+
+define void @ldpte_d(i64 %a) {
+; CHECK-LABEL: ldpte_d:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:ldpte $a0, 1
+; CHECK-NEXT:ret
+entry:
+  tail call void @llvm.loongarch.ldpte.d(i64 %a, i64 1)
+  ret void
+}
Index: llvm/test/CodeGen/LoongArch/intrinsic-la32-error.ll
===
--- llvm/test/CodeGen/LoongArch/intrinsic-la32-error.ll
+++ llvm/test/CodeGen/LoongArch/intrinsic-la32-error.ll
@@ -13,6 +13,10 @@
 declare i64 @llvm.loongarch.csrxchg.d(i64, i64, i32 immarg)
 declare i64 @llvm.loongarch.iocsrrd.d(i32)
 declare void @llvm.loongarch.iocsrwr.d(i64, i32)
+declare void @llvm.loongarch.asrtle.d(i64, i64)
+declare void @llvm.loongarch.asrtgt.d(i64, i64)
+declare i64 @llvm.loongarch.lddir.d(i64, i32)
+declare void @llvm.loongarch.ldpte.d(i64, i32)
 
 define i32 @crc_w_b_w(i32 %a, i32 %b) nounwind {
 ; CHECK: llvm.loongarch.crc.w.b.w requires target: loongarch64
@@ -104,3 +108,31 @@
   tail call void @llvm.loongarch.iocsrwr.d(i64 %a, i32 %b)
   ret void
 }
+
+define void @asrtle_d(i64 %a, i64 %b) {
+; CHECK: llvm.loongarch.asrtle.d requires target: loongarch64
+entry:
+  tail call void @llvm.loongarch.asrtle.d(i64 %a, i64 %b)
+  ret void
+}
+
+define void @asrtgt_d(i64 %a, i64 %b) {
+; CHECK: llvm.loongarch.asrtgt.d requires target: loongarch64
+entry:
+  tail call void @llvm.loongarch.asrtgt.d(i64 %a, i64 %b)
+  ret void
+}
+
+define i64 @lddir_d(i64 %a) {
+; CHECK: llvm.loongarch.lddir.d requires target: loongarch64
+entry:
+  %0 = tail call i64 @llvm.loongarch.lddir.d(i64 %a, i32 1)
+  ret i64 %0
+}
+
+define void @ldpte_d(i64 %a) {
+; CHECK: llvm.loongarch.ldpte.d requires target: loongarch64
+entry:
+  tail call void @llvm.loongarch.ldpte.d(i64 %a, i32 1)
+

[PATCH] D139915: [Clang][LoongArch] Add intrinsic for asrtle, asrtgt, lddir, ldpte and cpucfg

2022-12-12 Thread Gong LingQin via Phabricator via cfe-commits 
gonglingqin added a comment.

In D139915#3991089 , @xen0n wrote:

> Linux only requires `__cpucfg` among the ones you just added. Please amend 
> the patch summary so it's more accurate.
>
> Otherwise LGTM, thanks!

Thanks for your review, I will modify the summary.




Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:797
+ASRT_LE_GT_CASE(asrtle_d)
+ASRT_LE_GT_CASE(asrtgt_d)
+  case Intrinsic::loongarch_ldpte_d: {

xen0n wrote:
> `#undef ASRT_LE_GT_CASE` afterwards?
Thanks! I will modify it. And there are other similar problems in this file, 
which I will modify later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139915

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

[PATCH] D139915: [Clang][LoongArch] Add intrinsic for asrtle, asrtgt, lddir, ldpte and cpucfg

2022-12-12 Thread WÁNG Xuěruì via Phabricator via cfe-commits 
xen0n added a comment.

Linux only requires `__cpucfg` among the ones you just added. Please amend the 
patch summary so it's more accurate.

Otherwise LGTM, thanks!




Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:797
+ASRT_LE_GT_CASE(asrtle_d)
+ASRT_LE_GT_CASE(asrtgt_d)
+  case Intrinsic::loongarch_ldpte_d: {

`#undef ASRT_LE_GT_CASE` afterwards?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139915

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

[PATCH] D139915: [Clang][LoongArch] Add intrinsic for asrtle, asrtgt, lddir, ldpte and cpucfg

2022-12-12 Thread Gong LingQin via Phabricator via cfe-commits 
gonglingqin created this revision.
gonglingqin added reviewers: xen0n, xry111, SixWeining, wangleiat, MaskRay, 
XiaodongLoong.
Herald added subscribers: StephenFan, hiraditya.
Herald added a project: All.
gonglingqin requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

These intrinsics are required by Linux [1].

[1]: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/loongarch/include/asm/loongarch.h#n59


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139915

Files:
  clang/include/clang/Basic/BuiltinsLoongArch.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/larchintrin.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/LoongArch/intrinsic-la32-error.c
  clang/test/CodeGen/LoongArch/intrinsic-la32.c
  clang/test/CodeGen/LoongArch/intrinsic-la64-error.c
  clang/test/CodeGen/LoongArch/intrinsic-la64.c
  llvm/include/llvm/IR/IntrinsicsLoongArch.td
  llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp
  llvm/lib/Target/LoongArch/LoongArchISelLowering.h
  llvm/lib/Target/LoongArch/LoongArchInstrInfo.td
  llvm/test/CodeGen/LoongArch/intrinsic-la32-error.ll
  llvm/test/CodeGen/LoongArch/intrinsic-la64.ll
  llvm/test/CodeGen/LoongArch/intrinsic.ll

Index: llvm/test/CodeGen/LoongArch/intrinsic.ll
===
--- llvm/test/CodeGen/LoongArch/intrinsic.ll
+++ llvm/test/CodeGen/LoongArch/intrinsic.ll
@@ -15,6 +15,7 @@
 declare void @llvm.loongarch.iocsrwr.b(i32, i32)
 declare void @llvm.loongarch.iocsrwr.h(i32, i32)
 declare void @llvm.loongarch.iocsrwr.w(i32, i32)
+declare i32 @llvm.loongarch.cpucfg(i32)
 
 define void @foo() nounwind {
 ; CHECK-LABEL: foo:
@@ -145,3 +146,13 @@
   tail call void @llvm.loongarch.iocsrwr.w(i32 %a, i32 %b)
   ret void
 }
+
+define i32 @cpucfg(i32 %a) {
+; CHECK-LABEL: cpucfg:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:cpucfg $a0, $a0
+; CHECK-NEXT:ret
+entry:
+  %0 = tail call i32 @llvm.loongarch.cpucfg(i32 %a)
+  ret i32 %0
+}
Index: llvm/test/CodeGen/LoongArch/intrinsic-la64.ll
===
--- llvm/test/CodeGen/LoongArch/intrinsic-la64.ll
+++ llvm/test/CodeGen/LoongArch/intrinsic-la64.ll
@@ -14,6 +14,10 @@
 declare i64 @llvm.loongarch.csrxchg.d(i64, i64, i32 immarg)
 declare i64 @llvm.loongarch.iocsrrd.d(i32)
 declare void @llvm.loongarch.iocsrwr.d(i64, i32)
+declare void @llvm.loongarch.asrtle.d(i64, i64)
+declare void @llvm.loongarch.asrtgt.d(i64, i64)
+declare i64 @llvm.loongarch.lddir.d(i64, i64)
+declare void @llvm.loongarch.ldpte.d(i64, i64)
 
 define i32 @crc_w_b_w(i32 %a, i32 %b) nounwind {
 ; CHECK-LABEL: crc_w_b_w:
@@ -136,3 +140,43 @@
   tail call void @llvm.loongarch.iocsrwr.d(i64 %a, i32 %b)
   ret void
 }
+
+define void @asrtle_d(i64 %a, i64 %b) {
+; CHECK-LABEL: asrtle_d:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:asrtle.d $a0, $a1
+; CHECK-NEXT:ret
+entry:
+  tail call void @llvm.loongarch.asrtle.d(i64 %a, i64 %b)
+  ret void
+}
+
+define void @asrtgt_d(i64 %a, i64 %b) {
+; CHECK-LABEL: asrtgt_d:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:asrtgt.d $a0, $a1
+; CHECK-NEXT:ret
+entry:
+  tail call void @llvm.loongarch.asrtgt.d(i64 %a, i64 %b)
+  ret void
+}
+
+define i64 @lddir_d(i64 %a) {
+; CHECK-LABEL: lddir_d:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:lddir $a0, $a0, 1
+; CHECK-NEXT:ret
+entry:
+  %0 = tail call i64 @llvm.loongarch.lddir.d(i64 %a, i64 1)
+  ret i64 %0
+}
+
+define void @ldpte_d(i64 %a) {
+; CHECK-LABEL: ldpte_d:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:ldpte $a0, 1
+; CHECK-NEXT:ret
+entry:
+  tail call void @llvm.loongarch.ldpte.d(i64 %a, i64 1)
+  ret void
+}
Index: llvm/test/CodeGen/LoongArch/intrinsic-la32-error.ll
===
--- llvm/test/CodeGen/LoongArch/intrinsic-la32-error.ll
+++ llvm/test/CodeGen/LoongArch/intrinsic-la32-error.ll
@@ -13,6 +13,10 @@
 declare i64 @llvm.loongarch.csrxchg.d(i64, i64, i32 immarg)
 declare i64 @llvm.loongarch.iocsrrd.d(i32)
 declare void @llvm.loongarch.iocsrwr.d(i64, i32)
+declare void @llvm.loongarch.asrtle.d(i64, i64)
+declare void @llvm.loongarch.asrtgt.d(i64, i64)
+declare i64 @llvm.loongarch.lddir.d(i64, i32)
+declare void @llvm.loongarch.ldpte.d(i64, i32)
 
 define i32 @crc_w_b_w(i32 %a, i32 %b) nounwind {
 ; CHECK: llvm.loongarch.crc.w.b.w requires target: loongarch64
@@ -104,3 +108,31 @@
   tail call void @llvm.loongarch.iocsrwr.d(i64 %a, i32 %b)
   ret void
 }
+
+define void @asrtle_d(i64 %a, i64 %b) {
+; CHECK: llvm.loongarch.asrtle.d requires target: loongarch64
+entry:
+  tail call void @llvm.loongarch.asrtle.d(i64 %a, i64 %b)
+  ret void
+}
+
+define void @asrtgt_d(i64 %a, i64 %b) {
+; CHECK: llvm.loongarch.asrtgt.d requires target: loongarch64
+entry:
+  tail call void @llvm.loongarch.asrtgt.d(i64 %a, i64 %b)
+  ret void
+}
+
+define i

[PATCH] D139287: [OpenMP] Introduce basic JIT support to OpenMP target offloading

2022-12-12 Thread Johannes Doerfert via Phabricator via cfe-commits 
jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LG as there seem to be only nits left. We can expand on this in tree




Comment at: 
openmp/libomptarget/plugins-nextgen/common/PluginInterface/JIT.cpp:184
+
+  auto AddStream =
+  [&](size_t Task,

jhuber6 wrote:
> tianshilei1992 wrote:
> > jhuber6 wrote:
> > > tianshilei1992 wrote:
> > > > jhuber6 wrote:
> > > > > tianshilei1992 wrote:
> > > > > > Is there any way that we don't write it to a file here?
> > > > > Why do we need to invoke LTO here? I figured that we could call the 
> > > > > backend directly since we have no need to actually link any filies, 
> > > > > and we may not have a need to run more expensive optimizations when 
> > > > > the bitcode is already optimized. If you do that then you should be 
> > > > > able to just use a `raw_svector_ostream` as your output stream and 
> > > > > get the compiled output written to that buffer.
> > > > For the purpose of this basic JIT support, we indeed just need backend. 
> > > > However, since we have the plan for super optimization, etc., having an 
> > > > optimization pipeline here is also useful.
> > > We should be able to configure our own optimization pipeline in that 
> > > case, we might want the extra control as well.
> > which means we basically rewrite the function `opt` and `backend` in 
> > `LTO.cpp`. I thought about just invoking backend before, especially using 
> > LTO requires us to build the resolution table. However, after a second 
> > thought, I think it would be better to just use LTO.
> Building the passes isn't too complicated, it would take up the same amount 
> of space as the symbol resolutions and has the advantage that we don't need 
> to write the output to a file. I could write an implementation for this to 
> see how well it works.
Agreed. We can test it in a follow up and decide then.



Comment at: 
openmp/libomptarget/plugins-nextgen/common/PluginInterface/JIT.cpp:121
+
+CodeGenOpt::Level getCGOptLevel(unsigned OptLevel) {
+  switch (OptLevel) {

jhuber6 wrote:
> I'm tempted to move this into LLVM somewhere since it's been duplicated so 
> many times.
Let's do this as a follow up.



Comment at: openmp/libomptarget/plugins-nextgen/common/PluginInterface/JIT.h:41
+/// PostProcessing will be called after codegen to handle cases such as 
assember
+/// is an external tool.
+Expected<__tgt_device_image *> compile(__tgt_device_image *Image,




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139287

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

[PATCH] D137059: [Driver] [C++20] [Modules] Support -fmodule-output= (2/2)

2022-12-12 Thread Chuanqi Xu via Phabricator via cfe-commits 
ChuanqiXu updated this revision to Diff 482360.
ChuanqiXu added a comment.

Update as the dependent one changes.


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

https://reviews.llvm.org/D137059

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/module-output.cppm


Index: clang/test/Driver/module-output.cppm
===
--- clang/test/Driver/module-output.cppm
+++ clang/test/Driver/module-output.cppm
@@ -2,8 +2,14 @@
 // output and the name of the .pcm file should be the same with the output 
file.
 // RUN: %clang -std=c++20 %s -fmodule-output -c -o %t/output/Hello.o \
 // RUN:   -### 2>&1 | FileCheck %s
+//
+// Tests that the .pcm file will be generated in the same path with the 
specified one in the comamnd line.
+// RUN: %clang -std=c++20 %s -fmodule-output=%t/pcm/Hello.pcm -o %t/Hello.o \
+// RUN:   -c -### 2>&1 | FileCheck %s --check-prefix=CHECK-SPECIFIED
 
 export module Hello;
 
 // CHECK: "-emit-module-interface" {{.*}}"-main-file-name" 
"module-output.cppm" {{.*}}"-o" "{{.*}}/Hello.pcm" "-x" "c++" 
"{{.*}}/module-output.cppm"
 // CHECK: "-emit-obj" {{.*}}"-main-file-name" "module-output.cppm" {{.*}}"-o" 
"{{.*}}/Hello.o" "-x" "pcm" "{{.*}}/Hello.pcm"
+// CHECK-SPECIFIED: "-emit-module-interface" {{.*}}"-main-file-name" 
"module-output.cppm" {{.*}}"-o" "{{.*}}/Hello.pcm" "-x" "c++" 
"{{.*}}/module-output.cppm"
+// CHECK-SPECIFIED: "-emit-obj" {{.*}}"-main-file-name" "module-output.cppm" 
{{.*}}"-o" "{{.*}}/Hello.o" "-x" "pcm" "{{.*}}/Hello.pcm"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -5530,15 +5530,21 @@
   }
 
   // If `-fmodule-output` is specfied, then:
-  // - If `-o` is specified, the module file is writing to the same path
+  // - If `-fmodule-output` has a value, the module file is
+  //   writing to the value.
+  // - Else if `-o` is specified, the module file is writing to the same path
   //   with the output file in module file's suffix. 
-  // - If `-o` is not specified, the module file is writing to the same path
+  // - Else, the module file is writing to the same path
   //   with the input file in module file's suffix.
   if (!AtTopLevel && isa(JA) &&
   JA.getType() == types::TY_ModuleFile &&
-  C.getArgs().hasArg(options::OPT_fmodule_output)) {
+  (C.getArgs().hasArg(options::OPT_fmodule_output) ||
+   C.getArgs().hasArg(options::OPT_fmodule_output_EQ))) {
 SmallString<128> TempPath;
-if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o))
+
+if (Arg *ModuleFilePath = 
C.getArgs().getLastArg(options::OPT_fmodule_output_EQ))
+  TempPath = ModuleFilePath->getValue();
+else if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o))
   TempPath = FinalOutput->getValue();
 else
   TempPath = BaseInput;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2285,6 +2285,8 @@
   PosFlag,
   NegFlag, BothFlags<[NoXarchOption, CC1Option]>>;
 
+def fmodule_output_EQ : Joined<["-"], "fmodule-output=">, 
Flags<[NoXarchOption, CC1Option]>,
+  HelpText<"Save intermediate module file results when compiling a standard 
C++ module unit.">;
 def fmodule_output : Flag<["-"], "fmodule-output">, Flags<[NoXarchOption, 
CC1Option]>,
   HelpText<"Save intermediate module file results when compiling a standard 
C++ module unit.">;
 


Index: clang/test/Driver/module-output.cppm
===
--- clang/test/Driver/module-output.cppm
+++ clang/test/Driver/module-output.cppm
@@ -2,8 +2,14 @@
 // output and the name of the .pcm file should be the same with the output file.
 // RUN: %clang -std=c++20 %s -fmodule-output -c -o %t/output/Hello.o \
 // RUN:   -### 2>&1 | FileCheck %s
+//
+// Tests that the .pcm file will be generated in the same path with the specified one in the comamnd line.
+// RUN: %clang -std=c++20 %s -fmodule-output=%t/pcm/Hello.pcm -o %t/Hello.o \
+// RUN:   -c -### 2>&1 | FileCheck %s --check-prefix=CHECK-SPECIFIED
 
 export module Hello;
 
 // CHECK: "-emit-module-interface" {{.*}}"-main-file-name" "module-output.cppm" {{.*}}"-o" "{{.*}}/Hello.pcm" "-x" "c++" "{{.*}}/module-output.cppm"
 // CHECK: "-emit-obj" {{.*}}"-main-file-name" "module-output.cppm" {{.*}}"-o" "{{.*}}/Hello.o" "-x" "pcm" "{{.*}}/Hello.pcm"
+// CHECK-SPECIFIED: "-emit-module-interface" {{.*}}"-main-file-name" "module-output.cppm" {{.*}}"-o" "{{.*}}/Hello.pcm" "-x" "c++" "{{.*}}/module-output.cppm"
+// CHECK-SPECIFIED: "-emit-obj" {{.*}}"-main-file-name" "module-output.cppm" {{.*}}"-o" "{{.*}}/Hello.o" "-x" "pcm" "{{.*}}/Hello.pcm"
Index: clang/lib/Driver/Driver.cpp
==

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

2022-12-12 Thread Chuanqi Xu via Phabricator via cfe-commits 
ChuanqiXu added a comment.

> (I'd probably still reduce the test down to one example, using -o and 
> skipping the extra OUTPUT_PATH details, only checking the last part of the 
> path is as specified (or perhaps checking that it matches the .o file?))

Done.

> Also, could you consider the questions @urnathan asked in the 
> cross-project/flag naming thread about the semantics of this flag - I imagine 
> the right behavior is "whatever we do for .o files", but it'd be good to 
> check/consider/test them (they may not need automated testing - if the 
> behaviour is implemented with the same utilities for .o files as for .pcm 
> files then I don't mind just a statement that that's the case and some manual 
> testing has been done to verify that all works)?

Yeah, I've replied in that thread. And it looks like we can't test the 
behaviors if we can only check the output of `-###` in this patch. I've made 
some manual testing locally and let's document the behaviors later.




Comment at: clang/lib/Driver/Driver.cpp:5541-5545
+if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o))
+  TempPath = FinalOutput->getValue();
+else
+  TempPath = BaseInput;
+

dblaikie wrote:
> It'd be nice if we didn't have to recompute this/lookup `OPT_o` here - any 
> way we can use the object file output path here (that's already handled 
> `OPT_o` or using the base input name, etc)?
I didn't understand this a lot. We don't compute anything here and we just use 
the object file output path here if `-o` is provided and we replace the suffix 
then. I feel this is simple enough.



Comment at: clang/lib/Driver/Driver.cpp:5541-5542
+  //
+  // FIXME: Do we need to handle the case that the calculated output is
+  // conflicting with the specified output file or the input file?
+  if (!AtTopLevel && isa(JA) &&

dblaikie wrote:
> tahonermann wrote:
> > ChuanqiXu wrote:
> > > dblaikie wrote:
> > > > Do we do that for `-o` today? (eg: if you try to `-o` and specify the 
> > > > input file name, such that the output would overwrite the input, what 
> > > > happens? I'm not sure - but I guess it's OK to do whatever that is for 
> > > > this case too)
> > > > Do we do that for -o today? (eg: if you try to -o and specify the input 
> > > > file name, such that the output would overwrite the input, what 
> > > > happens? I'm not sure - but I guess it's OK to do whatever that is for 
> > > > this case too)
> > > 
> > > In this case, the input file will be overwrite and no warning shows. So 
> > > it looks like we didn't take special treatment here. So I remove the 
> > > FIXME.
> > Basing the location of the module output on the presence of `-o` sounds 
> > confusing to me. Likewise, defaulting to writing next to the input file as 
> > opposed to the current directory (where a `.o` file is written by default) 
> > sounds wrong. I think this option should be handled similarly to `-o`; it 
> > should accept a path and:
> >   - If an absolute path is provided, write to that location.
> >   - If a relative path is provided, write to that location relative to the 
> > current working directory.
> > Leave it up to the user or build system to ensure that the `.o` and `.pcm` 
> > file locations coincide if that is what they want. In general, I don't 
> > expect colocation of `.o` and `.pcm` files to be what is desired.
> > 
> > 
> @tahonermann there's precedent for this with Split DWARF (.dwo files go next 
> to the .o file) & I'd argued for possibly only providing this behavior, 
> letting consumers move files elsewhere if they needed to, but got voted down 
> there.
> 
> I do think this is a reasonable default, though. Users and build systems have 
> the option to pass a path to place the .pcm somewhere else (in the follow-up 
> patch to this one).
@tahonermann here is another patch which implements the behavior you described: 
https://reviews.llvm.org/D137059


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

https://reviews.llvm.org/D137058

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

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

2022-12-12 Thread Chuanqi Xu via Phabricator via cfe-commits 
ChuanqiXu updated this revision to Diff 482359.
ChuanqiXu marked an inline comment as done.
ChuanqiXu added a comment.

- Reduce the test down to one example, using -o and skipping the extra 
OUTPUT_PATH details - as the comment required.


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

https://reviews.llvm.org/D137058

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/lit.local.cfg
  clang/test/Driver/module-output.cppm


Index: clang/test/Driver/module-output.cppm
===
--- /dev/null
+++ clang/test/Driver/module-output.cppm
@@ -0,0 +1,9 @@
+// Tests that the .pcm file will be generated in the same direcotry with the 
specified 
+// output and the name of the .pcm file should be the same with the output 
file.
+// RUN: %clang -std=c++20 %s -fmodule-output -c -o %t/output/Hello.o \
+// RUN:   -### 2>&1 | FileCheck %s
+
+export module Hello;
+
+// CHECK: "-emit-module-interface" {{.*}}"-main-file-name" 
"module-output.cppm" {{.*}}"-o" "{{.*}}/Hello.pcm" "-x" "c++" 
"{{.*}}/module-output.cppm"
+// CHECK: "-emit-obj" {{.*}}"-main-file-name" "module-output.cppm" {{.*}}"-o" 
"{{.*}}/Hello.o" "-x" "pcm" "{{.*}}/Hello.pcm"
Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -1,6 +1,6 @@
 from lit.llvm import llvm_config
 
-config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', 
'.F90', '.f95',
+config.suffixes = ['.c', '.cpp', '.cppm', '.h', '.m', '.mm', '.S', '.s', 
'.f90', '.F90', '.f95',
'.cu', '.rs', '.cl', '.clcpp', '.hip', '.hlsl']
 config.substitutions = list(config.substitutions)
 config.substitutions.insert(0,
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -5529,6 +5529,25 @@
 return "-";
   }
 
+  // If `-fmodule-output` is specfied, then:
+  // - If `-o` is specified, the module file is writing to the same path
+  //   with the output file in module file's suffix. 
+  // - If `-o` is not specified, the module file is writing to the same path
+  //   with the input file in module file's suffix.
+  if (!AtTopLevel && isa(JA) &&
+  JA.getType() == types::TY_ModuleFile &&
+  C.getArgs().hasArg(options::OPT_fmodule_output)) {
+SmallString<128> TempPath;
+if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o))
+  TempPath = FinalOutput->getValue();
+else
+  TempPath = BaseInput;
+
+const char *Extension = types::getTypeTempSuffix(JA.getType());
+llvm::sys::path::replace_extension(TempPath, Extension);
+return C.addResultFile(C.getArgs().MakeArgString(TempPath.c_str()), &JA);
+  }
+
   if (IsDXCMode() && !C.getArgs().hasArg(options::OPT_o))
 return "-";
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2285,6 +2285,9 @@
   PosFlag,
   NegFlag, BothFlags<[NoXarchOption, CC1Option]>>;
 
+def fmodule_output : Flag<["-"], "fmodule-output">, Flags<[NoXarchOption, 
CC1Option]>,
+  HelpText<"Save intermediate module file results when compiling a standard 
C++ module unit.">;
+
 def fmodules_prune_interval : Joined<["-"], "fmodules-prune-interval=">, 
Group,
   Flags<[CC1Option]>, MetaVarName<"">,
   HelpText<"Specify the interval (in seconds) between attempts to prune the 
module cache">,


Index: clang/test/Driver/module-output.cppm
===
--- /dev/null
+++ clang/test/Driver/module-output.cppm
@@ -0,0 +1,9 @@
+// Tests that the .pcm file will be generated in the same direcotry with the specified 
+// output and the name of the .pcm file should be the same with the output file.
+// RUN: %clang -std=c++20 %s -fmodule-output -c -o %t/output/Hello.o \
+// RUN:   -### 2>&1 | FileCheck %s
+
+export module Hello;
+
+// CHECK: "-emit-module-interface" {{.*}}"-main-file-name" "module-output.cppm" {{.*}}"-o" "{{.*}}/Hello.pcm" "-x" "c++" "{{.*}}/module-output.cppm"
+// CHECK: "-emit-obj" {{.*}}"-main-file-name" "module-output.cppm" {{.*}}"-o" "{{.*}}/Hello.o" "-x" "pcm" "{{.*}}/Hello.pcm"
Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -1,6 +1,6 @@
 from lit.llvm import llvm_config
 
-config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
+config.suffixes = ['.c', '.cpp', '.cppm', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
'.cu', '.rs', '.cl', '.clcpp', '.hip', '.hlsl']
 config.substitutions = list(config.substitutions)
 config.substitutions.insert(0,
Ind

[clang] cd50f91 - [Clang][NFC] Prevent lit tests from matching substrings in current path

2022-12-12 Thread Sameer Sahasrabuddhe via cfe-commits 

Author: Sameer Sahasrabuddhe
Date: 2022-12-13T11:18:39+05:30
New Revision: cd50f910f4d1a6bb54fd8968f067febbc7320f28

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

LOG: [Clang][NFC] Prevent lit tests from matching substrings in current path

Added: 


Modified: 
clang/test/CodeGenCXX/2004-01-11-DynamicInitializedConstant.cpp
clang/test/CodeGenCXX/ignored-bitfield-conditional.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/2004-01-11-DynamicInitializedConstant.cpp 
b/clang/test/CodeGenCXX/2004-01-11-DynamicInitializedConstant.cpp
index 0c9333fb6d7a..0cd8419185ae 100644
--- a/clang/test/CodeGenCXX/2004-01-11-DynamicInitializedConstant.cpp
+++ b/clang/test/CodeGenCXX/2004-01-11-DynamicInitializedConstant.cpp
@@ -1,6 +1,12 @@
 // RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
 
+// Catch the beginning and the end of the IR. This prevents the CHECK-NOT from
+// matching a spurious "constant" string in file paths printed in the output.
+//
+// CHECK-LABEL: target triple
 // CHECK-NOT: constant
+// CHECK-LABEL: attributes
+
 extern int X;
 const int Y = X;
 const int* foo() { return &Y; }

diff  --git a/clang/test/CodeGenCXX/ignored-bitfield-conditional.cpp 
b/clang/test/CodeGenCXX/ignored-bitfield-conditional.cpp
index 7700e97ae9d5..d5763f49f954 100644
--- a/clang/test/CodeGenCXX/ignored-bitfield-conditional.cpp
+++ b/clang/test/CodeGenCXX/ignored-bitfield-conditional.cpp
@@ -7,7 +7,7 @@ struct S {
 };
 
 void use(bool cond, struct S s1, struct S s2, int val1, int val2) {
-  // CHECK: define {{.*}}use{{.*}}(
+  // CHECK-LABEL: define {{.*}}use{{.*}}(
   // CHECK: %[[S1:.+]] = alloca %struct.S
   // CHECK: %[[S2:.+]] = alloca %struct.S
   // CHECK: %[[COND:.+]] = alloca i8
@@ -86,7 +86,7 @@ void use(bool cond, struct S s1, struct S s2, int val1, int 
val2) {
 
 
 void use2(bool cond1, bool cond2, struct S s1, int val1, int val2, int val3) {
-  // CHECK: define {{.*}}use2{{.*}}(
+  // CHECK-LABEL: define {{.*}}use2{{.*}}(
   // CHECK: %[[S1:.+]] = alloca %struct.S
   // CHECK: %[[COND1:.+]] = alloca i8
   // CHECK: %[[COND2:.+]] = alloca i8
@@ -141,7 +141,11 @@ void use2(bool cond1, bool cond2, struct S s1, int val1, 
int val2, int val3) {
   // CHECK: store i16 %[[BF_SET]], ptr %[[S1]]
   // CHECK: br label %[[END:.+]]
 
-  // CHECK[[END]]:
+  // CHECK: [[END]]:
   // CHECK-NOT: phi
   // Nothing left to do here.
 }
+
+// Catch the end of the IR. This prevents the CHECK-NOT above from matching a
+// spurious "phi" in file paths printed in the output.
+// CHECK-LABEL: attributes



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

[PATCH] D138546: Clangd: Preserve target flags in system includes extractor

2022-12-12 Thread Christopher Sauer via Phabricator via cfe-commits 
cpsauer added a comment.

Sweet. Thanks again, Nathan.

Guys, lmk how you'd like to go from here. If you approve; feel free to copy 
in/land as part of the other change if that would save time. 
(I don't have commit access anyway.)


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

https://reviews.llvm.org/D138546

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

[PATCH] D139910: [NFC] Cleanup: Remove Function::getBasicBlockList() when not required.

2022-12-12 Thread Vasileios Porpodas via Phabricator via cfe-commits 
vporpo created this revision.
vporpo added reviewers: aeubanks, asbirlea.
Herald added subscribers: Moerafaat, zero9178, Enna1, bzcheeseman, mattd, 
awarzynski, sdasgup3, wenzhicui, wrengr, ormris, cota, teijeong, rdzhabarov, 
tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, 
mgester, arpith-jacob, antiagainst, shauheen, rriddle, mehdi_amini, hiraditya.
Herald added a reviewer: ftynse.
Herald added a project: All.
vporpo requested review of this revision.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, 
stephenneuendorffer, nicolasvasilache.
Herald added a reviewer: dcaballe.
Herald added projects: clang, LLDB, MLIR, LLVM.

This is part of a series of patches that aim at making 
Function::getBasicBlockList() private.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139910

Files:
  clang/lib/CodeGen/CGVTables.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptx86ABIFixups.cpp
  llvm/lib/ExecutionEngine/Orc/SpeculateAnalyses.cpp
  llvm/lib/Target/AArch64/SMEABIPass.cpp
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/lib/Transforms/IPO/InlineSimple.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/unittests/FuzzMutate/StrategiesTest.cpp
  llvm/unittests/IR/InstructionsTest.cpp
  mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

Index: mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
===
--- mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
+++ mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
@@ -321,8 +321,7 @@
   blocks.insert(traversal.begin(), traversal.end());
 }
   }
-  assert(blocks.size() == func->getBasicBlockList().size() &&
- "some blocks are not sorted");
+  assert(blocks.size() == func->size() && "some blocks are not sorted");
 
   return blocks;
 }
Index: llvm/unittests/IR/InstructionsTest.cpp
===
--- llvm/unittests/IR/InstructionsTest.cpp
+++ llvm/unittests/IR/InstructionsTest.cpp
@@ -1516,7 +1516,7 @@
 }
 )");
   Function *Foo = M->getFunction("foo");
-  auto BBs = Foo->getBasicBlockList().begin();
+  auto BBs = Foo->begin();
   CallBrInst &CBI = cast(BBs->front());
   ++BBs;
   ++BBs;
Index: llvm/unittests/FuzzMutate/StrategiesTest.cpp
===
--- llvm/unittests/FuzzMutate/StrategiesTest.cpp
+++ llvm/unittests/FuzzMutate/StrategiesTest.cpp
@@ -505,14 +505,14 @@
   std::unique_ptr M = parseAssembly(Source.data(), Ctx);
   Function *F = &*M->begin();
   DenseMap PreShuffleInstCnt;
-  for (BasicBlock &BB : F->getBasicBlockList()) {
+  for (BasicBlock &BB : *F) {
 PreShuffleInstCnt.insert({&BB, BB.size()});
   }
   std::mt19937 mt(Seed);
   std::uniform_int_distribution RandInt(INT_MIN, INT_MAX);
   for (int i = 0; i < 100; i++) {
 Mutator->mutateModule(*M, RandInt(mt), Source.size(), Source.size() + 1024);
-for (BasicBlock &BB : F->getBasicBlockList()) {
+for (BasicBlock &BB : *F) {
   int PostShuffleIntCnt = BB.size();
   EXPECT_EQ(PostShuffleIntCnt, PreShuffleInstCnt[&BB]);
 }
Index: llvm/lib/Transforms/Utils/InlineFunction.cpp
===
--- llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -2758,7 +2758,7 @@
 OrigBB->splice(CB.getIterator(), &*FirstNewBlock, FirstNewBlock->begin(),
FirstNewBlock->end());
 // Remove the cloned basic block.
-Caller->getBasicBlockList().pop_back();
+Caller->back().eraseFromParent();
 
 // If the call site was an invoke instruction, add a branch to the normal
 // destination.
@@ -2932,7 +2932,7 @@
   Br->eraseFromParent();
 
   // Now we can remove the CalleeEntry block, which is now empty.
-  Caller->getBasicBlockList().erase(CalleeEntry);
+  CalleeEntry->eraseFromParent();
 
   // If we inserted a phi node, check to see if it has a single value (e.g. all
   // the entries are the same or undef).  If so, remove the PHI so it doesn't
Index: llvm/lib/Transforms/Utils/FunctionComparator.cpp
===
--- llvm/lib/Transforms/Utils/FunctionComparator.cpp
+++ llvm/lib/Transforms/Utils/FunctionComparator.cpp
@@ -381,7 +381,7 @@
   BasicBlock *RBB = RBA->getBasicBlock();
   if (LBB == RBB)
 return 0;
-  for (BasicBlock &BB : F->getBasicBlockList()) {
+  for (BasicBlock &BB : *F) {
 if (&BB == LBB) {
   assert(&BB != RBB);
   return -1;
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Tran

[PATCH] D139906: [IR][NFC] Adds Function::insertBasicBlockAt() to replace things like F->getBasicBlockList().insert()

2022-12-12 Thread Vasileios Porpodas 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 rGa19ae77d2a90: [IR][NFC] Adds Function::insertBasicBlockAt() 
to replace things like F… (authored by vporpo).

Changed prior to commit:
  https://reviews.llvm.org/D139906?vs=482337&id=482348#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139906

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  llvm/examples/Kaleidoscope/BuildingAJIT/Chapter1/toy.cpp
  llvm/examples/Kaleidoscope/BuildingAJIT/Chapter2/toy.cpp
  llvm/examples/Kaleidoscope/BuildingAJIT/Chapter3/toy.cpp
  llvm/examples/Kaleidoscope/BuildingAJIT/Chapter4/toy.cpp
  llvm/examples/Kaleidoscope/Chapter5/toy.cpp
  llvm/examples/Kaleidoscope/Chapter6/toy.cpp
  llvm/examples/Kaleidoscope/Chapter7/toy.cpp
  llvm/examples/Kaleidoscope/Chapter8/toy.cpp
  llvm/examples/Kaleidoscope/Chapter9/toy.cpp
  llvm/include/llvm/IR/Function.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/IR/BasicBlock.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/Transforms/Utils/BreakCriticalEdges.cpp
  llvm/lib/Transforms/Utils/CloneFunction.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/lib/Transforms/Utils/LoopUnroll.cpp
  llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp
  llvm/lib/Transforms/Utils/LowerSwitch.cpp
  llvm/unittests/Analysis/FunctionPropertiesAnalysisTest.cpp
  llvm/unittests/IR/FunctionTest.cpp

Index: llvm/unittests/IR/FunctionTest.cpp
===
--- llvm/unittests/IR/FunctionTest.cpp
+++ llvm/unittests/IR/FunctionTest.cpp
@@ -7,12 +7,29 @@
 //===--===//
 
 #include "llvm/IR/Function.h"
+#include "llvm/AsmParser/Parser.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Support/SourceMgr.h"
 #include "gtest/gtest.h"
 using namespace llvm;
 
 namespace {
 
+static std::unique_ptr parseIR(LLVMContext &C, const char *IR) {
+  SMDiagnostic Err;
+  std::unique_ptr Mod = parseAssemblyString(IR, Err, C);
+  if (!Mod)
+Err.print("InstructionsTests", errs());
+  return Mod;
+}
+
+static BasicBlock *getBBWithName(Function *F, StringRef Name) {
+  auto It = find_if(
+  *F, [&Name](const BasicBlock &BB) { return BB.getName() == Name; });
+  assert(It != F->end() && "Not found!");
+  return &*It;
+}
+
 TEST(FunctionTest, hasLazyArguments) {
   LLVMContext C;
 
@@ -162,4 +179,64 @@
   EXPECT_EQ(Align(4), Func->getPointerAlignment(DataLayout("Fn32")));
 }
 
+TEST(FunctionTest, InsertBasicBlockAt) {
+  LLVMContext C;
+  std::unique_ptr M = parseIR(C, R"(
+define void @foo(i32 %a, i32 %b) {
+foo_bb0:
+  ret void
+}
+
+define void @bar() {
+bar_bb0:
+  br label %bar_bb1
+bar_bb1:
+  br label %bar_bb2
+bar_bb2:
+  ret void
+}
+)");
+  Function *FooF = M->getFunction("foo");
+  BasicBlock *FooBB0 = getBBWithName(FooF, "foo_bb0");
+
+  Function *BarF = M->getFunction("bar");
+  BasicBlock *BarBB0 = getBBWithName(BarF, "bar_bb0");
+  BasicBlock *BarBB1 = getBBWithName(BarF, "bar_bb1");
+  BasicBlock *BarBB2 = getBBWithName(BarF, "bar_bb2");
+
+  // Insert foo_bb0 into bar() at the very top.
+  FooBB0->removeFromParent();
+  auto It = BarF->insertBasicBlockAt(BarF->begin(), FooBB0);
+  EXPECT_EQ(BarBB0->getPrevNode(), FooBB0);
+  EXPECT_EQ(It, FooBB0->getIterator());
+
+  // Insert foo_bb0 into bar() at the very end.
+  FooBB0->removeFromParent();
+  It = BarF->insertBasicBlockAt(BarF->end(), FooBB0);
+  EXPECT_EQ(FooBB0->getPrevNode(), BarBB2);
+  EXPECT_EQ(FooBB0->getNextNode(), nullptr);
+  EXPECT_EQ(It, FooBB0->getIterator());
+
+  // Insert foo_bb0 into bar() just before bar_bb0.
+  FooBB0->removeFromParent();
+  It = BarF->insertBasicBlockAt(BarBB0->getIterator(), FooBB0);
+  EXPECT_EQ(FooBB0->getPrevNode(), nullptr);
+  EXPECT_EQ(FooBB0->getNextNode(), BarBB0);
+  EXPECT_EQ(It, FooBB0->getIterator());
+
+  // Insert foo_bb0 into bar() just before bar_bb1.
+  FooBB0->removeFromParent();
+  It = BarF->insertBasicBlockAt(BarBB1->getIterator(), FooBB0);
+  EXPECT_EQ(FooBB0->getPrevNode(), BarBB0);
+  EXPECT_EQ(FooBB0->getNextNode(), BarBB1);
+  EXPECT_EQ(It, FooBB0->getIterator());
+
+  // Insert foo_bb0 into bar() just before bar_bb2.
+  FooBB0->removeFromParent();
+  It = BarF->insertBasicBlockAt(BarBB2->getIterator(), FooBB0);
+  EXPECT_EQ(FooBB0->getPrevNode(), BarBB1);
+  EXPECT_EQ(FooBB0->getNextNode(), BarBB2);
+  EXPECT_EQ(It, FooBB0->getIterator());
+}
+
 } // end namespace
Index: llvm/unittests/Analysis/FunctionPropertiesAnalysisTest.cpp
===
--- llvm/unittests/Analysis/FunctionPropertiesAnalysisTest.cpp
+++ llvm/unittests/Analysis/FunctionPropertiesAnalysisTest.cpp
@@ -325,8 +325,7 @@
   EXPECT_TRUE(IR.isSuccess());
   invalidate(*F1);
   FPU.finish(FAM);
-  EXPECT_EQ(static_cast(FPI.BasicBlockCount),
-

[clang] a19ae77 - [IR][NFC] Adds Function::insertBasicBlockAt() to replace things like F->getBasicBlockList().insert()

2022-12-12 Thread Vasileios Porpodas via cfe-commits 

Author: Vasileios Porpodas
Date: 2022-12-12T20:22:55-08:00
New Revision: a19ae77d2a9016428fee7cd5af03fd20ad6d4464

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

LOG: [IR][NFC] Adds Function::insertBasicBlockAt() to replace things like 
F->getBasicBlockList().insert()

This is part of a series of patches that aim at making 
Function::getBasicBlockList() private.

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

Added: 


Modified: 
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
llvm/examples/Kaleidoscope/BuildingAJIT/Chapter1/toy.cpp
llvm/examples/Kaleidoscope/BuildingAJIT/Chapter2/toy.cpp
llvm/examples/Kaleidoscope/BuildingAJIT/Chapter3/toy.cpp
llvm/examples/Kaleidoscope/BuildingAJIT/Chapter4/toy.cpp
llvm/examples/Kaleidoscope/Chapter5/toy.cpp
llvm/examples/Kaleidoscope/Chapter6/toy.cpp
llvm/examples/Kaleidoscope/Chapter7/toy.cpp
llvm/examples/Kaleidoscope/Chapter8/toy.cpp
llvm/examples/Kaleidoscope/Chapter9/toy.cpp
llvm/include/llvm/IR/Function.h
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/lib/IR/BasicBlock.cpp
llvm/lib/IR/Core.cpp
llvm/lib/Transforms/Utils/BreakCriticalEdges.cpp
llvm/lib/Transforms/Utils/CloneFunction.cpp
llvm/lib/Transforms/Utils/CodeExtractor.cpp
llvm/lib/Transforms/Utils/LoopUnroll.cpp
llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp
llvm/lib/Transforms/Utils/LowerSwitch.cpp
llvm/unittests/Analysis/FunctionPropertiesAnalysisTest.cpp
llvm/unittests/IR/FunctionTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index c27254d6d3d62..f3264cf87552f 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -574,9 +574,9 @@ void CodeGenFunction::EmitBlock(llvm::BasicBlock *BB, bool 
IsFinished) {
   // Place the block after the current block, if possible, or else at
   // the end of the function.
   if (CurBB && CurBB->getParent())
-CurFn->getBasicBlockList().insertAfter(CurBB->getIterator(), BB);
+CurFn->insertBasicBlockAt(std::next(CurBB->getIterator()), BB);
   else
-CurFn->getBasicBlockList().push_back(BB);
+CurFn->insertBasicBlockAt(CurFn->end(), BB);
   Builder.SetInsertPoint(BB);
 }
 
@@ -601,15 +601,15 @@ void CodeGenFunction::EmitBlockAfterUses(llvm::BasicBlock 
*block) {
   bool inserted = false;
   for (llvm::User *u : block->users()) {
 if (llvm::Instruction *insn = dyn_cast(u)) {
-  CurFn->getBasicBlockList().insertAfter(insn->getParent()->getIterator(),
- block);
+  CurFn->insertBasicBlockAt(std::next(insn->getParent()->getIterator()),
+block);
   inserted = true;
   break;
 }
   }
 
   if (!inserted)
-CurFn->getBasicBlockList().push_back(block);
+CurFn->insertBasicBlockAt(CurFn->end(), block);
 
   Builder.SetInsertPoint(block);
 }
@@ -1469,7 +1469,7 @@ void CodeGenFunction::EmitCaseStmtRange(const CaseStmt &S,
   llvm::BasicBlock *FalseDest = CaseRangeBlock;
   CaseRangeBlock = createBasicBlock("sw.caserange");
 
-  CurFn->getBasicBlockList().push_back(CaseRangeBlock);
+  CurFn->insertBasicBlockAt(CurFn->end(), CaseRangeBlock);
   Builder.SetInsertPoint(CaseRangeBlock);
 
   // Emit range check.

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index a84f12938bc0c..09dc638a2ac32 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -319,8 +319,10 @@ llvm::DebugLoc CodeGenFunction::EmitReturnBlock() {
 
 static void EmitIfUsed(CodeGenFunction &CGF, llvm::BasicBlock *BB) {
   if (!BB) return;
-  if (!BB->use_empty())
-return CGF.CurFn->getBasicBlockList().push_back(BB);
+  if (!BB->use_empty()) {
+CGF.CurFn->insertBasicBlockAt(CGF.CurFn->end(), BB);
+return;
+  }
   delete BB;
 }
 

diff  --git a/llvm/examples/Kaleidoscope/BuildingAJIT/Chapter1/toy.cpp 
b/llvm/examples/Kaleidoscope/BuildingAJIT/Chapter1/toy.cpp
index 440d1b8dbd716..1f0036922b67d 100644
--- a/llvm/examples/Kaleidoscope/BuildingAJIT/Chapter1/toy.cpp
+++ b/llvm/examples/Kaleidoscope/BuildingAJIT/Chapter1/toy.cpp
@@ -863,7 +863,7 @@ Value *IfExprAST::codegen() {
   ThenBB = Builder->GetInsertBlock();
 
   // Emit else block.
-  TheFunction->getBasicBlockList().push_back(ElseBB);
+  TheFunction->insertBasicBlockAt(TheFunction->end(), ElseBB);
   Builder->SetInsertPoint(ElseBB);
 
   Value *ElseV = Else->codegen();
@@ -875,7 +875,7 @@ Value *IfExprAST::codegen() {
   ElseBB = Builder->GetInsertBlock();
 
   // Emit merge block.
-  TheFunction->getBasicBlockList().push_back(MergeBB);
+  TheFunction->insertBasicBlockAt(TheFunction->end(), MergeBB);
   Builder->SetInsertPoint(MergeBB);

[PATCH] D139908: [clang] Do not extend i8 return values to i16 on AVR.

2022-12-12 Thread Ben Shi via Phabricator via cfe-commits 
benshi001 added a comment.

This patch fixes https://github.com/llvm/llvm-project/issues/58877 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139908

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

[PATCH] D139908: [clang] Do not extend i8 return values to i16 on AVR.

2022-12-12 Thread Ben Shi via Phabricator via cfe-commits 
benshi001 created this revision.
benshi001 added reviewers: aykevl, dylanmckay.
Herald added a subscriber: Jim.
Herald added a project: All.
benshi001 requested review of this revision.
Herald added subscribers: cfe-commits, jacquesguan.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139908

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/avr/avr-builtins.c


Index: clang/test/CodeGen/avr/avr-builtins.c
===
--- clang/test/CodeGen/avr/avr-builtins.c
+++ clang/test/CodeGen/avr/avr-builtins.c
@@ -8,7 +8,7 @@
 return __builtin_bitreverse8(data);
 }
 
-// CHECK: define{{.*}} zeroext i8 @bitrev8
+// CHECK: define{{.*}} i8 @bitrev8
 // CHECK: i8 @llvm.bitreverse.i8(i8
 
 unsigned int bitrev16(unsigned int data) {
@@ -35,7 +35,7 @@
 return __builtin_rotateleft8(x, y);
 }
 
-// CHECK: define{{.*}} zeroext i8 @rotleft8
+// CHECK: define{{.*}} i8 @rotleft8
 // CHECK: i8 @llvm.fshl.i8(i8
 
 unsigned int rotleft16(unsigned int x, unsigned int y) {
@@ -62,7 +62,7 @@
 return __builtin_rotateright8(x, y);
 }
 
-// CHECK: define{{.*}} zeroext i8 @rotright8
+// CHECK: define{{.*}} i8 @rotright8
 // CHECK: i8 @llvm.fshr.i8(i8
 
 unsigned int rotright16(unsigned int x, unsigned int y) {
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -8435,6 +8435,10 @@
   LargeRet = true;
   return getNaturalAlignIndirect(Ty);
 }
+// An i8 return value should not be extended to i16, since AVR has 8-bit
+// registers.
+if (Ty->isIntegralOrEnumerationType() && getContext().getTypeSize(Ty) <= 8)
+  return ABIArgInfo::getDirect();
 // Otherwise we follow the default way which is compatible.
 return DefaultABIInfo::classifyReturnType(Ty);
   }


Index: clang/test/CodeGen/avr/avr-builtins.c
===
--- clang/test/CodeGen/avr/avr-builtins.c
+++ clang/test/CodeGen/avr/avr-builtins.c
@@ -8,7 +8,7 @@
 return __builtin_bitreverse8(data);
 }
 
-// CHECK: define{{.*}} zeroext i8 @bitrev8
+// CHECK: define{{.*}} i8 @bitrev8
 // CHECK: i8 @llvm.bitreverse.i8(i8
 
 unsigned int bitrev16(unsigned int data) {
@@ -35,7 +35,7 @@
 return __builtin_rotateleft8(x, y);
 }
 
-// CHECK: define{{.*}} zeroext i8 @rotleft8
+// CHECK: define{{.*}} i8 @rotleft8
 // CHECK: i8 @llvm.fshl.i8(i8
 
 unsigned int rotleft16(unsigned int x, unsigned int y) {
@@ -62,7 +62,7 @@
 return __builtin_rotateright8(x, y);
 }
 
-// CHECK: define{{.*}} zeroext i8 @rotright8
+// CHECK: define{{.*}} i8 @rotright8
 // CHECK: i8 @llvm.fshr.i8(i8
 
 unsigned int rotright16(unsigned int x, unsigned int y) {
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -8435,6 +8435,10 @@
   LargeRet = true;
   return getNaturalAlignIndirect(Ty);
 }
+// An i8 return value should not be extended to i16, since AVR has 8-bit
+// registers.
+if (Ty->isIntegralOrEnumerationType() && getContext().getTypeSize(Ty) <= 8)
+  return ABIArgInfo::getDirect();
 // Otherwise we follow the default way which is compatible.
 return DefaultABIInfo::classifyReturnType(Ty);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139395: Add CFI integer types normalization

2022-12-12 Thread Peter Collingbourne via Phabricator via cfe-commits 
pcc added a comment.

In D139395#3990772 , @rcvalle wrote:

> I elaborated on the reasons why not use a generalized encoding in the design 
> document in the tracking issue 
> https://github.com/rust-lang/rust/issues/89653. The tl;dr; is that it will 
> result in less comprehensive protection by either using a generalized 
> encoding for all C and C++ -compiled code or across the FFI boundary, and 
> will degrade the security of the program when linking foreign Rust-compiled 
> code into a program written in C or C++ because the program previously used a 
> more comprehensive encoding for all its compiled code, not fixing the issue 
> described in the design document and the RFC 
> https://github.com/rcvalle/rfcs/blob/improve-c-types-for-cross-language-cfi/text/-improve-c-types-for-cross-language-cfi.md#appendix.

Ack.




Comment at: clang/lib/AST/ItaniumMangle.cpp:2952
+  // uu
+  if (NormalizeIntegers && T->isInteger()) {
+if (T->isSignedInteger()) {

rcvalle wrote:
> pcc wrote:
> > `isInteger()` will return true for enums, but only if they are complete. 
> > This would mean that code such as
> > ```
> > void (*f)(enum E *e);
> > 
> > void g() {
> >   f(0);
> > }
> > ```
> > would use a different encoding to call `f` depending on whether the TU 
> > completes the enum `E`, if pointee types are considered part of the 
> > encoding.
> Isn't `isIntegerType()` that does that? `isInteger()` definition is:
> 
> ```
>   bool isInteger() const {
> return getKind() >= Bool && getKind() <= Int128;
>   }
> ```
Ah yes, sorry, somehow I read this as a call to `isIntegerType()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139395

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

[PATCH] D139395: Add CFI integer types normalization

2022-12-12 Thread Ramon de C Valle via Phabricator via cfe-commits 
rcvalle added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:2952
+  // uu
+  if (NormalizeIntegers && T->isInteger()) {
+if (T->isSignedInteger()) {

pcc wrote:
> `isInteger()` will return true for enums, but only if they are complete. This 
> would mean that code such as
> ```
> void (*f)(enum E *e);
> 
> void g() {
>   f(0);
> }
> ```
> would use a different encoding to call `f` depending on whether the TU 
> completes the enum `E`, if pointee types are considered part of the encoding.
Isn't `isIntegerType()` that does that? `isInteger()` definition is:

```
  bool isInteger() const {
return getKind() >= Bool && getKind() <= Int128;
  }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139395

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

[PATCH] D139287: [OpenMP] Introduce basic JIT support to OpenMP target offloading

2022-12-12 Thread Joseph Huber via Phabricator via cfe-commits 
jhuber6 added a comment.

Some nits.




Comment at: 
openmp/libomptarget/plugins-nextgen/common/PluginInterface/JIT.cpp:132
+
+OptimizationLevel getptLevel(unsigned OptLevel) {
+  switch (OptLevel) {

typo



Comment at: 
openmp/libomptarget/plugins-nextgen/common/PluginInterface/JIT.cpp:121
+
+CodeGenOpt::Level getCGOptLevel(unsigned OptLevel) {
+  switch (OptLevel) {

I'm tempted to move this into LLVM somewhere since it's been duplicated so many 
times.



Comment at: 
openmp/libomptarget/plugins-nextgen/common/PluginInterface/JIT.cpp:150
+Expected>
+createTargetMachine(Module &M, Triple TT, std::string CPU, unsigned OptLevel) {
+  CodeGenOpt::Level CGOptLevel = getCGOptLevel(OptLevel);

Why do we need `TT` if we expect to read the triple out of the Module?



Comment at: 
openmp/libomptarget/plugins-nextgen/common/PluginInterface/JIT.cpp:276-277
+
+  StringRef RawData(CGOutputBuffer.begin(), CGOutputBuffer.size());
+  return MemoryBuffer::getMemBufferCopy(RawData);
+}

Should work.



Comment at: 
openmp/libomptarget/plugins-nextgen/common/PluginInterface/JIT.cpp:301
+/// Output images generated from LLVM backend.
+std::list> JITImages;
+

Why a `std::list`? Since we use pointers we shouldn't need to worry about 
having a stable pointer and could use a `SmallVector` or similar right?



Comment at: 
openmp/libomptarget/plugins-nextgen/common/PluginInterface/JIT.cpp:319
+
+  StringRef Data((const char *)Image->ImageStart,
+ (char *)Image->ImageEnd - (char *)Image->ImageStart);

Should probably prefer C++ casts even if they are ridiculously verbose.



Comment at: 
openmp/libomptarget/plugins-nextgen/common/PluginInterface/JIT.cpp:343
+Expected<__tgt_device_image *> compile(__tgt_device_image *Image,
+   Triple::ArchType TA, std::string MCpu,
+   unsigned OptLevel,

`MCPU` is the more common version AFAICT.



Comment at: openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp:806
+
+  ///
+  struct ComputeCapabilityTy {

Missing comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139287

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

[PATCH] D139395: Add CFI integer types normalization

2022-12-12 Thread Ramon de C Valle via Phabricator via cfe-commits 
rcvalle added a comment.

I elaborated on the reasons why not use a generalized encoding in the design 
document in the tracking issue https://github.com/rust-lang/rust/issues/89653. 
The tl;dr; is that it will result in less comprehensive protection by either 
using a generalized encoding for all C and C++ -compiled code or across the FFI 
boundary, and will degrade the security of the program when linking foreign 
Rust-compiled code into a program written in C or C++ because the program 
previously used a more comprehensive encoding for all its compiled code, not 
fixing the issue described in the design document and the RFC 
https://github.com/rcvalle/rfcs/blob/improve-c-types-for-cross-language-cfi/text/-improve-c-types-for-cross-language-cfi.md#appendix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139395

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

[PATCH] D139906: [IR][NFC] Adds Function::insertBasicBlockAt() to replace things like F->getBasicBlockList().insert()

2022-12-12 Thread Vasileios Porpodas via Phabricator via cfe-commits 
vporpo created this revision.
vporpo added reviewers: aeubanks, asbirlea.
Herald added subscribers: zzheng, hiraditya.
Herald added a project: All.
vporpo requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This is part of a series of patches that aim at making 
Function::getBasicBlockList() private.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139906

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  llvm/examples/Kaleidoscope/BuildingAJIT/Chapter1/toy.cpp
  llvm/examples/Kaleidoscope/BuildingAJIT/Chapter2/toy.cpp
  llvm/examples/Kaleidoscope/BuildingAJIT/Chapter3/toy.cpp
  llvm/examples/Kaleidoscope/BuildingAJIT/Chapter4/toy.cpp
  llvm/examples/Kaleidoscope/Chapter5/toy.cpp
  llvm/examples/Kaleidoscope/Chapter6/toy.cpp
  llvm/examples/Kaleidoscope/Chapter7/toy.cpp
  llvm/examples/Kaleidoscope/Chapter8/toy.cpp
  llvm/examples/Kaleidoscope/Chapter9/toy.cpp
  llvm/include/llvm/IR/Function.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/IR/BasicBlock.cpp
  llvm/lib/IR/Core.cpp
  llvm/lib/Transforms/Utils/BreakCriticalEdges.cpp
  llvm/lib/Transforms/Utils/CloneFunction.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/lib/Transforms/Utils/LoopUnroll.cpp
  llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp
  llvm/lib/Transforms/Utils/LowerSwitch.cpp
  llvm/unittests/Analysis/FunctionPropertiesAnalysisTest.cpp
  llvm/unittests/IR/FunctionTest.cpp

Index: llvm/unittests/IR/FunctionTest.cpp
===
--- llvm/unittests/IR/FunctionTest.cpp
+++ llvm/unittests/IR/FunctionTest.cpp
@@ -7,12 +7,29 @@
 //===--===//
 
 #include "llvm/IR/Function.h"
+#include "llvm/AsmParser/Parser.h"
 #include "llvm/IR/Module.h"
+#include "llvm/Support/SourceMgr.h"
 #include "gtest/gtest.h"
 using namespace llvm;
 
 namespace {
 
+static std::unique_ptr parseIR(LLVMContext &C, const char *IR) {
+  SMDiagnostic Err;
+  std::unique_ptr Mod = parseAssemblyString(IR, Err, C);
+  if (!Mod)
+Err.print("InstructionsTests", errs());
+  return Mod;
+}
+
+static BasicBlock *getBBWithName(Function *F, StringRef Name) {
+  auto It = find_if(
+  *F, [&Name](const BasicBlock &BB) { return BB.getName() == Name; });
+  assert(It != F->end() && "Not found!");
+  return &*It;
+}
+
 TEST(FunctionTest, hasLazyArguments) {
   LLVMContext C;
 
@@ -162,4 +179,64 @@
   EXPECT_EQ(Align(4), Func->getPointerAlignment(DataLayout("Fn32")));
 }
 
+TEST(FunctionTest, InsertBasicBlockAt) {
+  LLVMContext C;
+  std::unique_ptr M = parseIR(C, R"(
+define void @foo(i32 %a, i32 %b) {
+foo_bb0:
+  ret void
+}
+
+define void @bar() {
+bar_bb0:
+  br label %bar_bb1
+bar_bb1:
+  br label %bar_bb2
+bar_bb2:
+  ret void
+}
+)");
+  Function *FooF = M->getFunction("foo");
+  BasicBlock *FooBB0 = getBBWithName(FooF, "foo_bb0");
+
+  Function *BarF = M->getFunction("bar");
+  BasicBlock *BarBB0 = getBBWithName(BarF, "bar_bb0");
+  BasicBlock *BarBB1 = getBBWithName(BarF, "bar_bb1");
+  BasicBlock *BarBB2 = getBBWithName(BarF, "bar_bb2");
+
+  // Insert foo_bb0 into bar() at the very top.
+  FooBB0->removeFromParent();
+  auto It = BarF->insertBasicBlockAt(BarF->begin(), FooBB0);
+  EXPECT_EQ(BarBB0->getPrevNode(), FooBB0);
+  EXPECT_EQ(It, FooBB0->getIterator());
+
+  // Insert foo_bb0 into bar() at the very end.
+  FooBB0->removeFromParent();
+  It = BarF->insertBasicBlockAt(BarF->end(), FooBB0);
+  EXPECT_EQ(FooBB0->getPrevNode(), BarBB2);
+  EXPECT_EQ(FooBB0->getNextNode(), nullptr);
+  EXPECT_EQ(It, FooBB0->getIterator());
+
+  // Insert foo_bb0 into bar() just before bar_bb0.
+  FooBB0->removeFromParent();
+  It = BarF->insertBasicBlockAt(BarBB0->getIterator(), FooBB0);
+  EXPECT_EQ(FooBB0->getPrevNode(), nullptr);
+  EXPECT_EQ(FooBB0->getNextNode(), BarBB0);
+  EXPECT_EQ(It, FooBB0->getIterator());
+
+  // Insert foo_bb0 into bar() just before bar_bb1.
+  FooBB0->removeFromParent();
+  It = BarF->insertBasicBlockAt(BarBB1->getIterator(), FooBB0);
+  EXPECT_EQ(FooBB0->getPrevNode(), BarBB0);
+  EXPECT_EQ(FooBB0->getNextNode(), BarBB1);
+  EXPECT_EQ(It, FooBB0->getIterator());
+
+  // Insert foo_bb0 into bar() just before bar_bb2.
+  FooBB0->removeFromParent();
+  It = BarF->insertBasicBlockAt(BarBB2->getIterator(), FooBB0);
+  EXPECT_EQ(FooBB0->getPrevNode(), BarBB1);
+  EXPECT_EQ(FooBB0->getNextNode(), BarBB2);
+  EXPECT_EQ(It, FooBB0->getIterator());
+}
+
 } // end namespace
Index: llvm/unittests/Analysis/FunctionPropertiesAnalysisTest.cpp
===
--- llvm/unittests/Analysis/FunctionPropertiesAnalysisTest.cpp
+++ llvm/unittests/Analysis/FunctionPropertiesAnalysisTest.cpp
@@ -325,8 +325,7 @@
   EXPECT_TRUE(IR.isSuccess());
   invalidate(*F1);
   FPU.finish(FAM);
-  EXPECT_EQ(static_cast(FPI.BasicBlockCount),
-F1->getBasicBlockList().size()

[PATCH] D139395: Add CFI integer types normalization

2022-12-12 Thread Peter Collingbourne via Phabricator via cfe-commits 
pcc added a reviewer: pcc.
pcc added a comment.

A high level question is whether we want to base the cross-language encoding on 
Itanium at all. Itanium has concepts such as substitutions that will complicate 
the implementation in other languages. Encoding pointee types can also lead to 
complications in cross-language encodings.

It may be better to consider developing a custom encoding. For the encoding 
being prototyped for the pointer authentication ABI on Android, the Rust side 
of the implementation is very simple:

https://github.com/pcc/rust/blob/d37ad119171635219ff21e054780d31024d24200/compiler/rustc_ty_utils/src/abi.rs#L396




Comment at: clang/lib/AST/ItaniumMangle.cpp:2952
+  // uu
+  if (NormalizeIntegers && T->isInteger()) {
+if (T->isSignedInteger()) {

`isInteger()` will return true for enums, but only if they are complete. This 
would mean that code such as
```
void (*f)(enum E *e);

void g() {
  f(0);
}
```
would use a different encoding to call `f` depending on whether the TU 
completes the enum `E`, if pointee types are considered part of the encoding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139395

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

[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

2022-12-12 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen added a comment.

In D139837#3989814 , @cor3ntin wrote:

> Hey. Thanks a lot for working on this.
> I did a first pass, looking at mostly style issues, looking at conformance 
> will probably take me a lot more time, but i think this looks pretty good 
> overall

Thanks for the quick fist-pass review. Appreciate it.




Comment at: clang/include/clang/AST/DeclCXX.h:1911
   setRangeEnd(EndLocation);
-setIsCopyDeductionCandidate(false);
+setDeductionCandidateKind(DeductionCandidateKind::Normal);
   }

cor3ntin wrote:
> I'm wondering if the constructor should take a `DeductionCandidateKind` 
> defaulted to normal here. All the places where it's set seem to be 
> immediately after construction.
That's true indeed. The awkward aspect is that the 
`CXXDeductionGuideDecl::Create` call is far from `setDeductionCandidateKind`; 
making `CXXDeductionGuideDecl::Create` takes a `DeductionCandidateKind` would 
several other less related functions takes `DeductionCandidateKind` also.



Comment at: clang/include/clang/AST/DeclCXX.h:1953-1959
   bool isCopyDeductionCandidate() const {
-return FunctionDeclBits.IsCopyDeductionCandidate;
+return getDeductionCandidateKind() == DeductionCandidateKind::Copy;
+  }
+
+  bool isAggregateDeductionCandidate() const {
+return getDeductionCandidateKind() == DeductionCandidateKind::Aggregate;
   }

cor3ntin wrote:
> I'm not sure how useful these things are, isAggregateDeductionCandidate is 
> only used once
I meant to make it consistent with `isCopyDeductionCandidate` which is also 
used once. Maybe remove both `isCopyDeductionCandidate` and 
`isAggregateDeductionCandidate`?



Comment at: clang/lib/Sema/SemaInit.cpp:312
   NoInitExpr *DummyExpr = nullptr;
+  SmallVector *AggrDeductionCandidateParamTypes = nullptr;
 

cor3ntin wrote:
> I think using optional here would make more sense, I guess that's why you 
> included it.
> Should it be SmallVectorImpl, such that only the caller has to bake in a size?
I meant to make `AggrDeductionCandidateParamTypes` a byref semantic like 
`FullyStructuredList`. So the  `InitListChecker` populates them as a 
side-effect. Using optional means the `InitListChecker` owes the deduced types 
which should work too but seems weird to me. 



Comment at: clang/lib/Sema/SemaInit.cpp:1098-1099
 maxElements = T->castAs()->getNumElements();
+  else if (T->isDependentType())
+maxElements = 1;
   else

cor3ntin wrote:
> Can you had a comment about that case? I'm not sure i understand what 
> scenario we are handling
Good point. On second thought, I think this is not needed. This function 
shouldn't accept dependent types.



Comment at: clang/lib/Sema/SemaInit.cpp:2167-2168
   // If we have any base classes, they are initialized prior to the fields.
-  for (auto &Base : Bases) {
+  for (auto I = Bases.begin(), E = Bases.end(); I != E; ++I) {
+auto &Base = *I;
 Expr *Init = Index < IList->getNumInits() ? IList->getInit(Index) : 
nullptr;

cor3ntin wrote:
> Maybe using `enumerate` + a range for loop here would be cleaner?
Using the iterator to test for the last element is easier. The index produced 
by `enumerate` would need to produce an iterator nevertheless.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139837

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

[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

2022-12-12 Thread Yuanfang Chen via Phabricator via cfe-commits 
ychen updated this revision to Diff 482331.
ychen marked 2 inline comments as done.
ychen added a comment.

- Address Corentin's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139837

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/TemplateDeduction.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/SemaTemplate/aggregate-deduction-candidate.cpp

Index: clang/test/SemaTemplate/aggregate-deduction-candidate.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/aggregate-deduction-candidate.cpp
@@ -0,0 +1,297 @@
+// RUN: %clang_cc1 -std=c++20 -verify -ast-dump -ast-dump-decl-types -ast-dump-filter "deduction guide" %s | FileCheck %s --strict-whitespace
+
+namespace Basic {
+  template struct A {
+T x;
+T y;
+  };
+
+  A a1{3.0, 4.0};
+  A a2{.x = 3.0, .y = 4.0};
+
+  // CHECK-LABEL: Dumping Basic:::
+  // CHECK: FunctionTemplateDecl {{.*}} implicit 
+  // CHECK: |-TemplateTypeParmDecl {{.*}} referenced class depth 0 index 0 T
+  // CHECK: |-CXXDeductionGuideDecl {{.*}} implicit  'auto (T, T) -> A'
+  // CHECK: | |-ParmVarDecl {{.*}} 'T'
+  // CHECK: | `-ParmVarDecl {{.*}} 'T'
+  // CHECK: `-CXXDeductionGuideDecl {{.*}} implicit used  'auto (double, double) -> Basic::A'
+  // CHECK:   |-TemplateArgument type 'double'
+  // CHECK:   | `-BuiltinType {{.*}} 'double'
+  // CHECK:   |-ParmVarDecl {{.*}} 'double':'double'
+  // CHECK:   `-ParmVarDecl {{.*}} 'double':'double'
+  // CHECK: FunctionProtoType {{.*}} 'auto (T, T) -> A' dependent trailing_return cdecl
+  // CHECK: |-InjectedClassNameType {{.*}} 'A' dependent
+  // CHECK: | `-CXXRecord {{.*}} 'A'
+  // CHECK: |-TemplateTypeParmType {{.*}} 'T' dependent depth 0 index 0
+  // CHECK: | `-TemplateTypeParm {{.*}} 'T'
+  // CHECK: `-TemplateTypeParmType {{.*}} 'T' dependent depth 0 index 0
+  // CHECK:   `-TemplateTypeParm {{.*}} 'T'
+
+  template  struct S {
+T x;
+T y;
+  };
+
+  template  struct C { // expected-note 5 {{candidate}}
+S s;
+T t;
+  };
+
+  template  struct D { // expected-note 3 {{candidate}}
+S s;
+T t;
+  };
+
+  C c1 = {1, 2}; // expected-error {{no viable}}
+  C c2 = {1, 2, 3}; // expected-error {{no viable}}
+  C c3 = {{1u, 2u}, 3};
+
+  D d1 = {1, 2}; // expected-error {{no viable}}
+  D d2 = {1, 2, 3};
+
+  // CHECK-LABEL: Dumping Basic:::
+  // CHECK: FunctionTemplateDecl {{.*}} implicit 
+  // CHECK: |-TemplateTypeParmDecl {{.*}} referenced typename depth 0 index 0 T
+  // CHECK: |-CXXDeductionGuideDecl {{.*}} implicit  'auto (S, T) -> C'
+  // CHECK: | |-ParmVarDecl {{.*}} 'S':'S'
+  // CHECK: | `-ParmVarDecl {{.*}} 'T'
+  // CHECK: `-CXXDeductionGuideDecl {{.*}} implicit used  'auto (S, int) -> Basic::C'
+  // CHECK:   |-TemplateArgument type 'int'
+  // CHECK:   | `-BuiltinType {{.*}} 'int'
+  // CHECK:   |-ParmVarDecl {{.*}} 'S':'Basic::S'
+  // CHECK:   `-ParmVarDecl {{.*}} 'int':'int'
+  // CHECK: FunctionProtoType {{.*}} 'auto (S, T) -> C' dependent trailing_return cdecl
+  // CHECK: |-InjectedClassNameType {{.*}} 'C' dependent
+  // CHECK: | `-CXXRecord {{.*}} 'C'
+  // CHECK: |-ElaboratedType {{.*}} 'S' sugar dependent
+  // CHECK: | `-TemplateSpecializationType {{.*}} 'S' dependent S
+  // CHECK: |   `-TemplateArgument type 'T'
+  // CHECK: | `-TemplateTypeParmType {{.*}} 'T' dependent depth 0 index 0
+  // CHECK: |   `-TemplateTypeParm {{.*}} 'T'
+  // CHECK: `-TemplateTypeParmType {{.*}} 'T' dependent depth 0 index 0
+  // CHECK:   `-TemplateTypeParm {{.*}} 'T'
+
+  // CHECK-LABEL: Dumping Basic:::
+  // CHECK: FunctionTemplateDecl {{.*}} implicit 
+  // CHECK: |-TemplateTypeParmDecl {{.*}} referenced typename depth 0 index 0 T
+  // CHECK: `-CXXDeductionGuideDecl {{.*}} implicit  'auto (int, int) -> D'
+  // CHECK:   |-ParmVarDecl {{.*}} 'int':'int'
+  // CHECK:   `-ParmVarDecl {{.*}} 'int':'int'
+  // CHECK: FunctionProtoType {{.*}} 'auto (int, int) -> D' dependent trailing_return cdecl
+  // CHECK: |-InjectedClassNameType {{.*}} 'D' dependent
+  // CHECK: | `-CXXRecord {{.*}} 'D'
+  // CHECK: |-SubstTemplateTypeParmType {{.*}} 'int' sugar typename depth 0 index 0 T
+  // CHECK: | |-ClassTemplateSpecialization {{.*}} 'S'
+  // CHECK: | `-BuiltinType {{.*}} 'int'
+  // CHECK: `-SubstTemplateTypeParmType {{.*}} 'int' sugar typename depth 0 index 0 T
+  // CHECK:   |-ClassTemplateSpecialization {{.*}} 'S'
+  // CHECK:   `-BuiltinType {{.*}} 'int'
+
+  template  struct E {
+T t;
+decltype(t) t2;
+  };
+
+  E e1 = {1, 2};
+
+  // CHECK-LABEL

[PATCH] D139450: Warn about unsupported ibmlongdouble

2022-12-12 Thread Qiu Chaofan via Phabricator via cfe-commits 
qiucf added inline comments.



Comment at: clang/test/Driver/lit.local.cfg:26
+if config.ppc_linux_default_ieeelongdouble == "ON":
+  config.available_features.add('ppc_linux_default_ieeelongdouble')

tuliom wrote:
> qiucf wrote:
> > Can we assume if we are compiling with `-mabi=ieeelongdouble`, then libc++ 
> > 'must' be built with the same long double ABI? If I understand correctly, 
> > they're unrelated.
> @qiucf I didn't understand this part. Are you suggesting to remove the long 
> double warnings because the way the compiler was built is unrelated to the 
> way libc++ was built?
Ah, I misunderstood meaning of 'defaults to IEEE long double' in last review. 
We can assume 'how the compiler was built' is the same as 'how the libc++ was 
built'.

But when 'how the libc++ was built' conflicts with 'how the compiler compiles 
current program', we expect a warning, right? If so, this looks reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139450

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

[PATCH] D139045: [HIP] support --offload-arch=native

2022-12-12 Thread Yaxun Liu via Phabricator via cfe-commits 
yaxunl marked 2 inline comments as done.
yaxunl added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:3058
   // Collect all offload arch parameters, removing duplicates.
+  const StringRef NativeArchStr = "native";
   std::set GpuArchs;

tra wrote:
> Nit: It's used only once. May as well compare with "native" directly.
will do



Comment at: clang/lib/Driver/Driver.cpp:3076
+SmallVector GPUs;
+auto Err = TC->detectSystemGPUs(Args, GPUs);
+if (!Err) {

tra wrote:
> Does it guarantee that a returned list of architectures contains only 
> canonical names?
Yes. amdgpu-arch calls ROCm runtime to get GPU arch, which uses canonical gfx 
names 
https://github.com/RadeonOpenCompute/ROCR-Runtime/blob/adae6c61e10d371f7cbc3d0e94ae2c070cab18a4/src/core/runtime/isa.cpp#L245


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

https://reviews.llvm.org/D139045

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

[PATCH] D139295: [Coroutines] Don't mark the parameter attribute of resume function as noalias

2022-12-12 Thread Chuanqi Xu 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 rG7dccdd76b374: [Coroutines] Don't mark the parameter 
attribute of resume function as noalias (authored by ChuanqiXu).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139295

Files:
  clang/test/CodeGenCoroutines/pr59221.cpp
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/test/Transforms/Coroutines/coro-debug-dbg.addr.ll
  llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll
  llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll
  llvm/test/Transforms/Coroutines/coro-debug.ll
  llvm/test/Transforms/Coroutines/coro-frame.ll

Index: llvm/test/Transforms/Coroutines/coro-frame.ll
===
--- llvm/test/Transforms/Coroutines/coro-frame.ll
+++ llvm/test/Transforms/Coroutines/coro-frame.ll
@@ -44,7 +44,7 @@
 ; CHECK: ret i8* %hdl
 
 ; See if the float was loaded from the frame
-; CHECK-LABEL: @f.resume(%f.Frame* noalias nonnull align 8
+; CHECK-LABEL: @f.resume(%f.Frame* noundef nonnull align 8
 ; CHECK: %r.reload = load double, double* %r.reload.addr
 ; CHECK: call double @print(double %r.reload)
 ; CHECK: ret void
Index: llvm/test/Transforms/Coroutines/coro-debug.ll
===
--- llvm/test/Transforms/Coroutines/coro-debug.ll
+++ llvm/test/Transforms/Coroutines/coro-debug.ll
@@ -172,7 +172,7 @@
 !32 = !DILocalVariable(name: "inline_asm", scope: !6, file: !7, line: 55, type: !11)
 
 ; CHECK: define i8* @f(i32 %x) #0 personality i32 0 !dbg ![[ORIG:[0-9]+]]
-; CHECK: define internal fastcc void @f.resume(%f.Frame* noalias nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[RESUME:[0-9]+]]
+; CHECK: define internal fastcc void @f.resume(%f.Frame* noundef nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[RESUME:[0-9]+]]
 ; CHECK: entry.resume:
 ; CHECK: %[[DBG_PTR:.*]] = alloca %f.Frame*
 ; CHECK: call void @llvm.dbg.declare(metadata %f.Frame** %[[DBG_PTR]], metadata ![[RESUME_COROHDL:[0-9]+]], metadata !DIExpression(DW_OP_deref, DW_OP_plus_uconst,
@@ -194,8 +194,8 @@
 ; CHECK-NEXT: to label %[[DEFAULT_DEST:.+]] [label
 ; CHECK: [[DEFAULT_DEST]]:
 ; CHECK-NEXT: call void @llvm.dbg.declare(metadata i32 %[[CALLBR_RES]]
-; CHECK: define internal fastcc void @f.destroy(%f.Frame* noalias nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[DESTROY:[0-9]+]]
-; CHECK: define internal fastcc void @f.cleanup(%f.Frame* noalias nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[CLEANUP:[0-9]+]]
+; CHECK: define internal fastcc void @f.destroy(%f.Frame* noundef nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[DESTROY:[0-9]+]]
+; CHECK: define internal fastcc void @f.cleanup(%f.Frame* noundef nonnull align 8 dereferenceable(40) %FramePtr) #0 personality i32 0 !dbg ![[CLEANUP:[0-9]+]]
 
 ; CHECK: ![[ORIG]] = distinct !DISubprogram(name: "f", linkageName: "flink"
 
Index: llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll
===
--- llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll
+++ llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll
@@ -2,7 +2,7 @@
 ; RUN: opt < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s
 ;
 ; This file is based on coro-debug-frame-variable.ll.
-; CHECK:  define internal fastcc void @f.resume(%f.Frame* noalias nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[RESUME_FN_DBG_NUM:[0-9]+]]
+; CHECK:  define internal fastcc void @f.resume(%f.Frame* noundef nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[RESUME_FN_DBG_NUM:[0-9]+]]
 ; CHECK:   init.ready:
 ; CHECK: call void @llvm.dbg.value(metadata %f.Frame** %FramePtr.debug, metadata ![[XVAR_RESUME:[0-9]+]],
 ; CHECK:   await.ready:
Index: llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll
===
--- llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll
+++ llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll
@@ -2,7 +2,7 @@
 ; RUN: opt < %s -passes='module(coro-early),cgscc(coro-split,coro-split)' -S | FileCheck %s
 ;
 ; This file is based on coro-debug-frame-variable.ll.
-; CHECK:  define internal fastcc void @f.resume(%f.Frame* noalias nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[RESUME_FN_DBG_NUM:[0-9]+]]
+; CHECK:  define internal fastcc void @f.resume(%f.Frame* noundef nonnull align 16 dereferenceable(80) %FramePtr) !dbg ![[RESUME_FN_DBG_NUM:[0-9]+]]
 ; CHECK:   await.ready:
 ; CHECK: call void @llvm.db

[clang] 7dccdd7 - [Coroutines] Don't mark the parameter attribute of resume function as noalias

2022-12-12 Thread Chuanqi Xu via cfe-commits 

Author: Chuanqi Xu
Date: 2022-12-13T10:19:24+08:00
New Revision: 7dccdd76b3749508f4a4147b0ba1f7f2689bebb2

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

LOG: [Coroutines] Don't mark the parameter attribute of resume function as 
noalias

Close https://github.com/llvm/llvm-project/issues/59221.

The root cause for the problem is that we marked the parameter of the
resume/destroy functions as noalias previously. But this is not true.

See https://github.com/llvm/llvm-project/issues/59221 for the details.
Long story short, for this C++ program
(https://compiler-explorer.com/z/6qGcozG93), the optimized frame will be
something like:

```
struct test_frame {
void (*__resume_)(), // a function pointer points to the
`test.resume` function, which can be imaged as the test() function in
the example.

struct a_frame {
  ...
  void **caller; // may points to test_frame at runtime.
};
};
```

And the function a and function test looks just like:

```
define i32 @a(ptr noalias %alloc_8) {
  %alloc_8_16 = getelementptr ptr, ptr %alloc_8, i64 16
  store i32 42, ptr %alloc_8_16, align 8
  %alloc_8_8 = getelementptr ptr, ptr %alloc_8, i64 8
  %alloc = load ptr, ptr %alloc_8_8, align 8
  %p = load ptr, ptr %alloc, align 8
  %r = call i32 %p(ptr %alloc)
  ret i32 %r
}

define i32 @b(ptr %p) {
entry:
  %alloc = alloca [128 x i8], align 8
  %alloc_8 = getelementptr ptr, ptr %alloc, i64 8
  %alloc_8_8 = getelementptr ptr, ptr %alloc_8, i64 8
  store ptr %alloc, ptr %alloc_8_8, align 8
  store ptr %p, ptr %alloc, align 8
  %r = call i32 @a(ptr nonnull %alloc_8)
  ret i32 %r
}
```

Here inside the function `a`,  we can access the parameter `%alloc_8` by
`%alloc` and we pass `%alloc` to an unknown function. So it breaks the
assumption of `noalias` parameter.

Note that although only CoroElide optimization can put a frame inside
another frame directly, the following case is not valid too:

```
struct test_frame {

   void **a_frame; // may points to a_frame at runtime.
};

 struct a_frame {
void **caller; // may points to test_frame at runtime.
};
```

Since the C++ language allows the programmer to get the address of
coroutine frames, we can't assume the above case wouldn't happen in the
source codes. So we can't set the parameter as noalias no matter if
CoroElide applies or not. And for other languages, it may be safe if
they don't allow the programmers to get the address of coroutine frames.

Reviewed By: nikic

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

Added: 
clang/test/CodeGenCoroutines/pr59221.cpp

Modified: 
llvm/lib/Transforms/Coroutines/CoroSplit.cpp
llvm/test/Transforms/Coroutines/coro-debug-dbg.addr.ll
llvm/test/Transforms/Coroutines/coro-debug-dbg.values-not_used_in_frame.ll
llvm/test/Transforms/Coroutines/coro-debug-dbg.values.ll
llvm/test/Transforms/Coroutines/coro-debug.ll
llvm/test/Transforms/Coroutines/coro-frame.ll

Removed: 




diff  --git a/clang/test/CodeGenCoroutines/pr59221.cpp 
b/clang/test/CodeGenCoroutines/pr59221.cpp
new file mode 100644
index ..ae5f6fdbdea9
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/pr59221.cpp
@@ -0,0 +1,88 @@
+// Test for PR59221. Tests the compiler wouldn't misoptimize the final result.
+//
+// REQUIRES: x86-registered-target
+//
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 %s -O3 -S 
-emit-llvm -o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+template  struct task {
+   struct promise_type {
+   T value{123};
+   std::coroutine_handle<> caller{std::noop_coroutine()};
+   
+   struct final_awaiter: std::suspend_always {
+   auto await_suspend(std::coroutine_handle 
me) const noexcept {
+   return me.promise().caller;
+   }
+   };
+
+   constexpr auto initial_suspend() const noexcept {
+   return std::suspend_always();
+   }
+   constexpr auto final_suspend() const noexcept {
+   return final_awaiter{};
+   }
+   auto unhandled_exception() noexcept {
+   // ignore
+   }
+   constexpr void return_value(T v) noexcept {
+   value = v;
+   } 
+   constexpr auto & get_return_object() noexcept {
+   return *this;
+   }
+   };
+   
+   using coroutine_handle = std::coroutine_handle;
+   
+   promise_type & promise{nullptr};
+   
+   task(promise_type & p) noexcept: promise{p} { }
+   
+   ~task() noexcept {
+   coroutine_handle::from_promise(p

[PATCH] D139395: Add CFI integer types normalization

2022-12-12 Thread Nick Desaulniers via Phabricator via cfe-commits 
nickdesaulniers added subscribers: hjl.tools, rjmccall.
nickdesaulniers added a comment.

Thanks for the patch and the work on cross language CFI support!

I wonder if we have precedent for other non-standard extensions to 
`ItaniumMangleContextImpl`?  I wonder if we should perhaps have a distinct 
subclass to denote that this is not the standard mangling scheme.  It would be 
nice perhaps to get this standardized in 
https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling.
https://github.com/itanium-cxx-abi/cxx-abi

cc @rjmccall @hjl.tools


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139395

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

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

2022-12-12 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5541-5545
+if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o))
+  TempPath = FinalOutput->getValue();
+else
+  TempPath = BaseInput;
+

It'd be nice if we didn't have to recompute this/lookup `OPT_o` here - any way 
we can use the object file output path here (that's already handled `OPT_o` or 
using the base input name, etc)?



Comment at: clang/lib/Driver/Driver.cpp:5541-5542
+  //
+  // FIXME: Do we need to handle the case that the calculated output is
+  // conflicting with the specified output file or the input file?
+  if (!AtTopLevel && isa(JA) &&

tahonermann wrote:
> ChuanqiXu wrote:
> > dblaikie wrote:
> > > Do we do that for `-o` today? (eg: if you try to `-o` and specify the 
> > > input file name, such that the output would overwrite the input, what 
> > > happens? I'm not sure - but I guess it's OK to do whatever that is for 
> > > this case too)
> > > Do we do that for -o today? (eg: if you try to -o and specify the input 
> > > file name, such that the output would overwrite the input, what happens? 
> > > I'm not sure - but I guess it's OK to do whatever that is for this case 
> > > too)
> > 
> > In this case, the input file will be overwrite and no warning shows. So it 
> > looks like we didn't take special treatment here. So I remove the FIXME.
> Basing the location of the module output on the presence of `-o` sounds 
> confusing to me. Likewise, defaulting to writing next to the input file as 
> opposed to the current directory (where a `.o` file is written by default) 
> sounds wrong. I think this option should be handled similarly to `-o`; it 
> should accept a path and:
>   - If an absolute path is provided, write to that location.
>   - If a relative path is provided, write to that location relative to the 
> current working directory.
> Leave it up to the user or build system to ensure that the `.o` and `.pcm` 
> file locations coincide if that is what they want. In general, I don't expect 
> colocation of `.o` and `.pcm` files to be what is desired.
> 
> 
@tahonermann there's precedent for this with Split DWARF (.dwo files go next to 
the .o file) & I'd argued for possibly only providing this behavior, letting 
consumers move files elsewhere if they needed to, but got voted down there.

I do think this is a reasonable default, though. Users and build systems have 
the option to pass a path to place the .pcm somewhere else (in the follow-up 
patch to this one).


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

https://reviews.llvm.org/D137058

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

[PATCH] D139395: Add CFI integer types normalization

2022-12-12 Thread Ramon de C Valle via Phabricator via cfe-commits 
rcvalle updated this revision to Diff 482327.
rcvalle added a comment.

Added ".normalized" suffix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139395

Files:
  clang/docs/ControlFlowIntegrity.rst
  clang/docs/UsersManual.rst
  clang/include/clang/AST/Mangle.h
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/CodeGen/cfi-icall-normalize.c

Index: clang/test/CodeGen/cfi-icall-normalize.c
===
--- /dev/null
+++ clang/test/CodeGen/cfi-icall-normalize.c
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-icall -fsanitize-trap=cfi-icall -fsanitize-cfi-icall-normalize-integers -emit-llvm -o - %s | FileCheck %s
+
+// Test that integer types are normalized for cross-language CFI support with
+// other languages that can't represent and encode C/C++ integer types.
+
+char foo(char (*fn)(char), char arg) {
+  // CHECK-LABEL: define{{.*}}foo
+  // CHECK-SAME: {{.*}}!type ![[TYPE1:[0-9]+]] !type ![[TYPE2:[0-9]+]]
+  // CHECK: call i1 @llvm.type.test(ptr {{.*}}, metadata !"_ZTSFu2i8S_E.normalized")
+  return fn(arg);
+}
+
+short bar(int (*fn)(short, int), short arg1, int arg2) {
+  // CHECK-LABEL: define{{.*}}bar
+  // CHECK-SAME: {{.*}}!type ![[TYPE3:[0-9]+]] !type ![[TYPE4:[0-9]+]]
+  // CHECK: call i1 @llvm.type.test(ptr {{.*}}, metadata !"_ZTSFu3i32u3i16S_E.normalized")
+  return (short)fn(arg1, arg2);
+}
+
+long long baz(__int128 (*fn)(long, long long, __int128), long arg1, long long arg2, __int128 arg3) {
+  // CHECK-LABEL: define{{.*}}baz
+  // CHECK-SAME: {{.*}}!type ![[TYPE5:[0-9]+]] !type ![[TYPE6:[0-9]+]]
+  // CHECK: call i1 @llvm.type.test(ptr {{.*}}, metadata !"_ZTSFu4i128u3i64S0_S_E.normalized")
+  return (long long)fn(arg1, arg2, arg3);
+}
+
+unsigned char foo1(unsigned char (*fn)(unsigned char), unsigned char arg) {
+  // CHECK-LABEL: define{{.*}}foo1
+  // CHECK-SAME: {{.*}}!type ![[TYPE7:[0-9]+]] !type ![[TYPE8:[0-9]+]]
+  // CHECK: call i1 @llvm.type.test(ptr {{.*}}, metadata !"_ZTSFu2u8S_E.normalized")
+  return fn(arg);
+}
+
+unsigned short bar1(unsigned int (*fn)(unsigned short, unsigned int), unsigned short arg1, unsigned int arg2) {
+  // CHECK-LABEL: define{{.*}}bar1
+  // CHECK-SAME: {{.*}}!type ![[TYPE9:[0-9]+]] !type ![[TYPE10:[0-9]+]]
+  // CHECK: call i1 @llvm.type.test(ptr {{.*}}, metadata !"_ZTSFu3u32u3u16S_E.normalized")
+  return (unsigned short)fn(arg1, arg2);
+}
+
+unsigned long long baz1(unsigned __int128 (*fn)(unsigned long, unsigned long long, unsigned __int128), unsigned long arg1, unsigned long long arg2, unsigned __int128 arg3) {
+  // CHECK-LABEL: define{{.*}}baz1
+  // CHECK-SAME: {{.*}}!type ![[TYPE11:[0-9]+]] !type ![[TYPE12:[0-9]+]]
+  // CHECK: call i1 @llvm.type.test(ptr {{.*}}, metadata !"_ZTSFu4u128u3u64S0_S_E.normalized")
+  return (unsigned long long)fn(arg1, arg2, arg3);
+}
+
+// CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFu2i8PFS_S_ES_E.normalized"}
+// CHECK: ![[TYPE2]] = !{i64 0, !"_ZTSFu2i8PvS_E.normalized.generalized"}
+// CHECK: ![[TYPE3]] = !{i64 0, !"_ZTSFu3i16PFu3i32S_S0_ES_S0_E.normalized"}
+// CHECK: ![[TYPE4]] = !{i64 0, !"_ZTSFu3i16PvS_u3i32E.normalized.generalized"}
+// CHECK: ![[TYPE5]] = !{i64 0, !"_ZTSFu3i64PFu4i128S_S_S0_ES_S_S0_E.normalized"}
+// CHECK: ![[TYPE6]] = !{i64 0, !"_ZTSFu3i64PvS_S_u4i128E.normalized.generalized"}
+// CHECK: ![[TYPE7]] = !{i64 0, !"_ZTSFu2u8PFS_S_ES_E.normalized"}
+// CHECK: ![[TYPE8]] = !{i64 0, !"_ZTSFu2u8PvS_E.normalized.generalized"}
+// CHECK: ![[TYPE9]] = !{i64 0, !"_ZTSFu3u16PFu3u32S_S0_ES_S0_E.normalized"}
+// CHECK: ![[TYPE10]] = !{i64 0, !"_ZTSFu3u16PvS_u3u32E.normalized.generalized"}
+// CHECK: ![[TYPE11]] = !{i64 0, !"_ZTSFu3u64PFu4u128S_S_S0_ES_S_S0_E.normalized"}
+// CHECK: ![[TYPE12]] = !{i64 0, !"_ZTSFu3u64PvS_S_u4u128E.normalized.generalized"}
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -715,6 +715,9 @@
 CfiICallGeneralizePointers =
 Args.hasArg(options::OPT_fsanitize_cfi_icall_generalize_pointers);
 
+CfiICallNormalizeIntegers =
+Args.hasArg(options::OPT_fsanitize_cfi_icall_normalize_integers);
+
 if (CfiCrossDso && CfiICallGeneralizePointers && DiagnoseErrors)
   D.Diag(diag::err_drv_argument_not_allowed_with)
   << "-fsanitize-cfi-cross-dso"
@@ -1218,6 +1221,9 @@
   if (CfiICallGeneralizePointers)
 CmdArgs.push_back("-fsanitize-cfi-icall-generalize-pointers");
 
+  if (CfiICallNormalizeIntegers)
+CmdArgs.push_back("-fsanitize-cfi-icall-normalize-integers");
+
   if (CfiCanonicalJumpTables)
 CmdArgs.

[clang] f7a1f7a - Fix test on 32-bit targets.

2022-12-12 Thread Richard Smith via cfe-commits 

Author: Richard Smith
Date: 2022-12-12T17:38:51-08:00
New Revision: f7a1f7ab70eadd8264db2d3f956a1a6bab749c01

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

LOG: Fix test on 32-bit targets.

Added: 


Modified: 
clang/test/CodeGenCXX/weak-external.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/weak-external.cpp 
b/clang/test/CodeGenCXX/weak-external.cpp
index 34af2028a9d3..af636ccb4a3a 100644
--- a/clang/test/CodeGenCXX/weak-external.cpp
+++ b/clang/test/CodeGenCXX/weak-external.cpp
@@ -90,9 +90,9 @@ namespace constant_eval {
 [[gnu::weak]] void f();
   };
   // CHECK-LABEL: define {{.*}} @__cxx_global_var_init
-  // CHECK: store i8 zext (i1 icmp ne (i64 ptrtoint (ptr 
@_ZN13constant_eval1X1fEv to i64), i64 0) to i8), ptr 
@_ZN13constant_eval6has_f1E,
+  // CHECK: store i8 zext (i1 icmp ne (i{{32|64}} ptrtoint (ptr 
@_ZN13constant_eval1X1fEv to i{{32|64}}), i{{32|64}} 0) to i8), ptr 
@_ZN13constant_eval6has_f1E,
   bool has_f1 = &X::f;
   // CHECK-LABEL: define {{.*}} @__cxx_global_var_init
-  // CHECK: store i8 zext (i1 icmp ne (i64 ptrtoint (ptr 
@_ZN13constant_eval1X1fEv to i64), i64 0) to i8), ptr 
@_ZN13constant_eval6has_f2E,
+  // CHECK: store i8 zext (i1 icmp ne (i{{32|64}} ptrtoint (ptr 
@_ZN13constant_eval1X1fEv to i{{32|64}}), i{{32|64}} 0) to i8), ptr 
@_ZN13constant_eval6has_f2E,
   bool has_f2 = &X::f != nullptr;
 }



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

[clang] 5982b0b - Add missing check for constant evaluation of a comparison of a pointer

2022-12-12 Thread Richard Smith via cfe-commits 

Author: Richard Smith
Date: 2022-12-12T17:09:26-08:00
New Revision: 5982b0b0b84296e34057a777e3d51e10fcd8abc7

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

LOG: Add missing check for constant evaluation of a comparison of a pointer
to member naming a weak member to nullptr.

This fixes a miscompile where constant evaluation would incorrectly
determine that a weak member function pointer is never null.

In passing, also improve the diagnostics for constant evaluation of some
nearby cases.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticASTKinds.td
clang/lib/AST/ExprConstant.cpp
clang/test/AST/Interp/cxx20.cpp
clang/test/CXX/drs/dr0xx.cpp
clang/test/CXX/drs/dr16xx.cpp
clang/test/CXX/expr/expr.const/p2-0x.cpp
clang/test/CodeGenCXX/weak-external.cpp
clang/test/Sema/const-eval.c
clang/test/SemaCXX/attr-weak.cpp
clang/test/SemaCXX/builtins.cpp
clang/test/SemaCXX/constant-expression-cxx11.cpp
clang/www/cxx_dr_status.html
clang/www/make_cxx_dr_status

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 53155b0e2a492..3aef8c93f6ffe 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -321,6 +321,9 @@ Bug Fixes
   `Issue 59100 `_
 - Fix issue using __attribute__((format)) on non-variadic functions that expect
   more than one formatted argument.
+- Fix bug where constant evaluation treated a pointer to member that points to
+  a weak member as never being null. Such comparisons are now treated as
+  non-constant.
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index 6a2d15ef77c6a..d4303ef834c17 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -84,7 +84,23 @@ def note_constexpr_pointer_subtraction_not_same_array : Note<
 def note_constexpr_pointer_subtraction_zero_size : Note<
   "subtraction of pointers to type %0 of zero size">;
 def note_constexpr_pointer_comparison_unspecified : Note<
-  "comparison has unspecified value">;
+  "comparison between '%0' and '%1' has unspecified value">;
+def note_constexpr_pointer_constant_comparison : Note<
+  "comparison of numeric address '%0' with pointer '%1' can only be performed "
+  "at runtime">;
+def note_constexpr_literal_comparison : Note<
+  "comparison of addresses of literals has unspecified value">;
+def note_constexpr_pointer_weak_comparison : Note<
+  "comparison against address of weak declaration '%0' can only be performed "
+  "at runtime">;
+def note_constexpr_mem_pointer_weak_comparison : Note<
+  "comparison against pointer to weak member %q0 can only be performed "
+  "at runtime">;
+def note_constexpr_pointer_comparison_past_end : Note<
+  "comparison against pointer '%0' that points past the end of a "
+  "complete object has unspecified value">;
+def note_constexpr_pointer_comparison_zero_sized : Note<
+  "comparison of pointers '%0' and '%1' to unrelated zero-sized objects">;
 def note_constexpr_pointer_comparison_base_classes : Note<
   "comparison of addresses of subobjects of 
diff erent base classes "
   "has unspecified value">;

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 280b37adc0321..8fd46197af334 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -2483,6 +2483,7 @@ static bool EvalPointerValueAsBool(const APValue &Value, 
bool &Result) {
   // A null base expression indicates a null pointer.  These are always
   // evaluatable, and they are false unless the offset is zero.
   if (!Value.getLValueBase()) {
+// TODO: Should a non-null pointer with an offset of zero evaluate to true?
 Result = !Value.getLValueOffset().isZero();
 return true;
   }
@@ -2495,6 +2496,7 @@ static bool EvalPointerValueAsBool(const APValue &Value, 
bool &Result) {
 }
 
 static bool HandleConversionToBool(const APValue &Val, bool &Result) {
+  // TODO: This function should produce notes if it fails.
   switch (Val.getKind()) {
   case APValue::None:
   case APValue::Indeterminate:
@@ -2519,6 +2521,9 @@ static bool HandleConversionToBool(const APValue &Val, 
bool &Result) {
   case APValue::LValue:
 return EvalPointerValueAsBool(Val, Result);
   case APValue::MemberPointer:
+if (Val.getMemberPointerDecl() && Val.getMemberPointerDecl()->isWeak()) {
+  return false;
+}
 Result = Val.getMemberPointerDecl();
 return true;
   case APValue::Vector:
@@ -12938,41 +12943,55 @@ EvaluateComparisonBinaryOperator(EvalInfo &Info, 
const B

[PATCH] D139749: Headers: make a couple of builtins non-static

2022-12-12 Thread Igor Zhukov via Phabricator via cfe-commits 
fsb4000 accepted this revision.
fsb4000 added a comment.
This revision is now accepted and ready to land.

I also asked at MS STL Discord(https://discord.gg/XWanNww) about the patch.
F25588619: изображение.png 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139749

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

[PATCH] D138861: [Clang] Implement CWG2640 Allow more characters in an n-char sequence

2022-12-12 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin added a comment.

https://reviews.llvm.org/D139889


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138861

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

[PATCH] D139889: [Clang] Fix a crash when encountering an ill-formed delimited UCN.

2022-12-12 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin created this revision.
Herald added a project: All.
cor3ntin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

\u{...} was incorrectly parsed as a valid UCN instead
of emitting a diagnostic, causing an assertion failure.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139889

Files:
  clang/lib/Lex/Lexer.cpp
  clang/test/Preprocessor/ucn-pp-identifier.c


Index: clang/test/Preprocessor/ucn-pp-identifier.c
===
--- clang/test/Preprocessor/ucn-pp-identifier.c
+++ clang/test/Preprocessor/ucn-pp-identifier.c
@@ -118,6 +118,7 @@
 // CHECK-NEXT: {{^   u}}
 
 #define \u{}   // expected-warning {{empty delimited universal 
character name; treating as '\' 'u' '{' '}'}} expected-error {{macro name must 
be an identifier}}
+#define \u1{123}   // expected-warning {{incomplete universal character 
name; treating as '\' followed by identifier}} expected-error {{macro name must 
be an identifier}}
 #define \u{123456789}  // expected-error {{hex escape sequence out of range}} 
expected-error {{macro name must be an identifier}}
 #define \u{// expected-warning {{incomplete delimited universal 
character name; treating as '\' 'u' '{' identifier}} expected-error {{macro 
name must be an identifier}}
 #define \u{fgh}// expected-warning {{incomplete delimited universal 
character name; treating as '\' 'u' '{' identifier}} expected-error {{macro 
name must be an identifier}}
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -3224,7 +3224,7 @@
   uint32_t CodePoint = 0;
   while (Count != NumHexDigits || Delimited) {
 char C = getCharAndSize(CurPtr, CharSize);
-if (!Delimited && C == '{') {
+if (!Delimited && Count == 0 && C == '{') {
   Delimited = true;
   CurPtr += CharSize;
   continue;


Index: clang/test/Preprocessor/ucn-pp-identifier.c
===
--- clang/test/Preprocessor/ucn-pp-identifier.c
+++ clang/test/Preprocessor/ucn-pp-identifier.c
@@ -118,6 +118,7 @@
 // CHECK-NEXT: {{^   u}}
 
 #define \u{}   // expected-warning {{empty delimited universal character name; treating as '\' 'u' '{' '}'}} expected-error {{macro name must be an identifier}}
+#define \u1{123}   // expected-warning {{incomplete universal character name; treating as '\' followed by identifier}} expected-error {{macro name must be an identifier}}
 #define \u{123456789}  // expected-error {{hex escape sequence out of range}} expected-error {{macro name must be an identifier}}
 #define \u{// expected-warning {{incomplete delimited universal character name; treating as '\' 'u' '{' identifier}} expected-error {{macro name must be an identifier}}
 #define \u{fgh}// expected-warning {{incomplete delimited universal character name; treating as '\' 'u' '{' identifier}} expected-error {{macro name must be an identifier}}
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -3224,7 +3224,7 @@
   uint32_t CodePoint = 0;
   while (Count != NumHexDigits || Delimited) {
 char C = getCharAndSize(CurPtr, CharSize);
-if (!Delimited && C == '{') {
+if (!Delimited && Count == 0 && C == '{') {
   Delimited = true;
   CurPtr += CharSize;
   continue;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136554: Implement CWG2631

2022-12-12 Thread Dan McGregor via Phabricator via cfe-commits 
dankm added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:9688
+}
+return llvm::None;
+  }

llvm::None has been deprecated in favour of std::nullopt.


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] D138861: [Clang] Implement CWG2640 Allow more characters in an n-char sequence

2022-12-12 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin added a comment.

In D138861#3988954 , @tahonermann 
wrote:

> @cor3ntin, I suspect the answer is no, but do your changes perhaps address 
> this assertion failure? https://godbolt.org/z/1bPcPcWzv
>
>   int \u1{234};

Interesting bug. 
No, it doesn't. But i see what's going on here, it's pretty easy to fix!




Comment at: clang/lib/Lex/Lexer.cpp:3323
 if (Diagnose)
   Diag(StartPtr, diag::warn_ucn_escape_incomplete);
 return llvm::None;

tahonermann wrote:
> tahonermann wrote:
> > I was able to confirm that `StartPtr` does point to `N`. See the diagnostic 
> > generated at https://godbolt.org/z/qnajcMeso; the diagnostic caret points 
> > to `N` instead of `\`.
> >   :1:2: warning: incomplete delimited universal character name; 
> > treating as '\' 'N' '{' identifier [-Wunicode]
> >   \N{abc
> >^
> I think this still needs to be addressed. `tryReadNumericUCN()` handles this 
> case by requiring that callers pass the location of the `\` as `SlashLoc`. I 
> guess this is technically required since the `\` character could be written 
> as either `\` or the `??/` trigraph.
Done (by adding SlashLoc)!
There is still room for improvement here but if we wanted to clean that up 
further we'd need a different patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138861

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

[PATCH] D138861: [Clang] Implement CWG2640 Allow more characters in an n-char sequence

2022-12-12 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin updated this revision to Diff 482279.
cor3ntin added a comment.

Fix caret position
Fix tests (defining a macro named 'a' caused issues)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138861

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Lex/Lexer.h
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/test/CXX/drs/dr26xx.cpp
  clang/test/Lexer/char-escapes-delimited.c
  clang/test/Lexer/unicode.c
  clang/test/Preprocessor/ucn-pp-identifier.c
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -15647,7 +15647,7 @@
 https://wg21.link/cwg2640";>2640
 accepted
 Allow more characters in an n-char sequence
-Unknown
+Clang 16
   
   
 https://wg21.link/cwg2641";>2641
Index: clang/test/Preprocessor/ucn-pp-identifier.c
===
--- clang/test/Preprocessor/ucn-pp-identifier.c
+++ clang/test/Preprocessor/ucn-pp-identifier.c
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 %s -fsyntax-only -std=c99 -pedantic -verify=expected,ext -Wundef
-// RUN: %clang_cc1 %s -fsyntax-only -x c++ -pedantic -verify=expected,ext -Wundef
-// RUN: %clang_cc1 %s -fsyntax-only -x c++ -std=c++2b -pedantic -ftrigraphs -verify=expected,cxx2b -Wundef -Wpre-c++2b-compat
+// RUN: %clang_cc1 %s -fsyntax-only -std=c99 -pedantic -verify=expected,ext -Wundef -DTRIGRAPHS=1
+// RUN: %clang_cc1 %s -fsyntax-only -x c++ -pedantic -verify=expected,ext -Wundef -fno-trigraphs
+// RUN: %clang_cc1 %s -fsyntax-only -x c++ -std=c++2b -pedantic -ftrigraphs -DTRIGRAPHS=1 -verify=expected,cxx2b -Wundef -Wpre-c++2b-compat
 // RUN: %clang_cc1 %s -fsyntax-only -x c++ -pedantic -verify=expected,ext -Wundef -ftrigraphs -DTRIGRAPHS=1
 // RUN: not %clang_cc1 %s -fsyntax-only -std=c99 -pedantic -Wundef 2>&1 | FileCheck -strict-whitespace %s
 
@@ -40,7 +40,6 @@
// ext-warning {{extension}} cxx2b-warning {{before C++2b}}
 #define \N{WASTEBASKET} // expected-error {{macro name must be an identifier}} \
 // ext-warning {{extension}} cxx2b-warning {{before C++2b}}
-
 #define a\u0024
 
 #if \u0110 // expected-warning {{is not defined, evaluates to 0}}
@@ -121,20 +120,39 @@
 #define \u{123456789}  // expected-error {{hex escape sequence out of range}} expected-error {{macro name must be an identifier}}
 #define \u{// expected-warning {{incomplete delimited universal character name; treating as '\' 'u' '{' identifier}} expected-error {{macro name must be an identifier}}
 #define \u{fgh}// expected-warning {{incomplete delimited universal character name; treating as '\' 'u' '{' identifier}} expected-error {{macro name must be an identifier}}
-#define \N{// expected-warning {{incomplete delimited universal character name; treating as '\' 'N' '{' identifier}} expected-error {{macro name must be an identifier}}
+#define \N{
+// expected-warning@-1 {{incomplete delimited universal character name; treating as '\' 'N' '{' identifier}}
+// expected-error@-2 {{macro name must be an identifier}}
 #define \N{}   // expected-warning {{empty delimited universal character name; treating as '\' 'N' '{' '}'}} expected-error {{macro name must be an identifier}}
 #define \N{NOTATHING}  // expected-error {{'NOTATHING' is not a valid Unicode character name}} \
// expected-error {{macro name must be an identifier}}
 #define \NN// expected-warning {{incomplete universal character name; treating as '\' followed by identifier}} expected-error {{macro name must be an identifier}}
 #define \N{GREEK_SMALL-LETTERALPHA}  // expected-error {{'GREEK_SMALL-LETTERALPHA' is not a valid Unicode character name}} \
  // expected-note {{characters names in Unicode escape sequences are sensitive to case and whitespaces}}
+#define \N{🤡}  // expected-error {{'🤡' is not a valid Unicode character name}} \
+// expected-error {{macro name must be an identifier}}
 
 #define CONCAT(A, B) A##B
-int CONCAT(\N{GREEK, CAPITALLETTERALPHA}); // expected-error{{expected}} \
-   // expected-warning {{incomplete delimited universal character name}}
+int CONCAT(\N{GREEK
+, CAPITALLETTERALPHA});
+// expected-error@-2 {{expected}} \
+// expected-warning@-2 {{incomplete delimited universal character name}}
+
+int \N{\
+LATIN CAPITAL LETTER A WITH GRAVE};
+//ext-warning@-2 {{extension}} cxx2b-warning@-2 {{before C++2b}}
 
 #ifdef TRIGRAPHS
-int \N?? = 0; // expected-warning{{extension}} cxx2b-warning {{before C++2b}} \
+int \N?? = 0; // cxx2b-warning {{before C++2b}} \
+//ext-warning {{extension}}\

[PATCH] D139398: [AMDGPU] Add bf16 storage support

2022-12-12 Thread Matt Arsenault via Phabricator via cfe-commits 
arsenm added inline comments.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:4819-4831
+// When we don't have 16 bit instructions, bf16 is illegal and gets
+// softened to i16 for storage, with float being used for arithmetic.
+//
+// After softening, some i16 -> fp32 bf16_to_fp operations can be left 
over.
+// Lower those to (f32 (fp_extend (f16 (bitconvert x
+if (!Op->getValueType(0).isFloatingPoint() ||
+Op->getOperand(0).getValueType() != MVT::i16)

Pierre-vh wrote:
> arsenm wrote:
> > Pierre-vh wrote:
> > > arsenm wrote:
> > > > Pierre-vh wrote:
> > > > > arsenm wrote:
> > > > > > Pierre-vh wrote:
> > > > > > > arsenm wrote:
> > > > > > > > Pierre-vh wrote:
> > > > > > > > > arsenm wrote:
> > > > > > > > > > The generic legalizer should have handled this?
> > > > > > > > > It looks like those operations are not implemented in the 
> > > > > > > > > generic legalizer, e.g. I get 
> > > > > > > > > ``` 
> > > > > > > > > Do not know how to promote this operator's operand!
> > > > > > > > > ```
> > > > > > > > Right, this is the code that would go there
> > > > > > > Do I just copy/paste this code in that PromoteInt function, and 
> > > > > > > keep a copy here too in LowerOperation? (not really a fan of 
> > > > > > > copy-pasting code in different files, I'd rather keep it all here)
> > > > > > > We need to have the lowering too AFAIK, it didn't go well when I 
> > > > > > > tried to remove it
> > > > > > I'm not following why you need to handle it here
> > > > > IIRC:
> > > > >  - I need to handle FP_TO_BF16 in ReplaceNodeResult because that's 
> > > > > what the Integer Legalizer calls (through CustomLowerNode)
> > > > >  - I need to handle both opcodes in LowerOperation because otherwise 
> > > > > they'll fail selection. They can be left over from 
> > > > > expanding/legalizing other operations.
> > > > But why are they custom? We don't have to handle FP16_TO_FP or 
> > > > FP_TO_FP16 there, and they aren't custom lowered. They have the same 
> > > > basic properties. We have this:
> > > > 
> > > > 
> > > > ```
> > > > setOperationAction(ISD::FP16_TO_FP, MVT::i16, Promote);
> > > > AddPromotedToType(ISD::FP16_TO_FP, MVT::i16, MVT::i32);
> > > > setOperationAction(ISD::FP_TO_FP16, MVT::i16, Promote);
> > > > AddPromotedToType(ISD::FP_TO_FP16, MVT::i16, MVT::i32);
> > > > ```
> > > > 
> > > > I'd expect the same basic pattern
> > > PromoteIntegerOperand, PromoteFloatOperand and PromoteIntegerResult don't 
> > > handle FP_TO_BF16 and BF16_TO_FP, and unless we put a Custom lowering 
> > > mode it'll assert/unreachable.
> > > I tried to make it work (for a while) using the default expand but I 
> > > can't quite get it to work. It feels like there is some legalizer work 
> > > missing for handling BF16 like we want to.
> > > Even though it's not ideal I think the custom lowering is easiest
> > What about Expand? that's where the implemented part is
> Last I tried, Expand will emit a libcall in many cases that we don't handle
Library call is supposed to be a distinct action now, the DAG only did about 5% 
of the work to migrate to using it. This code can go to the default expand 
action



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:5564
+  SDLoc SL(Op);
+  const TargetLowering &TLI = DAG.getTargetLoweringInfo();
+

This is just this



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:5578
+   DAG.getConstant(16, SL,
+   TLI.getShiftAmountTy(MVT::i32, DAG.getDataLayout()))});
+  return DAG.getNode(ISD::TRUNCATE, SL, MVT::i16, Result);

can just hardcode i32



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:5590
+  SDLoc SL(Op);
+  const TargetLowering &TLI = DAG.getTargetLoweringInfo();
+

This is just this



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:5596
+  ISD::SHL, SL, MVT::i32,
+  {DAG.getNode(ISD::ZERO_EXTEND, SL, MVT::i32, Op->getOperand(0)),
+   DAG.getConstant(16, SL,

This can be any_extend and the combiner will probably turn it into one



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:5598
+   DAG.getConstant(16, SL,
+   TLI.getShiftAmountTy(MVT::i32, DAG.getDataLayout()))});
+  Result = DAG.getBitcast(MVT::f32, Result);

Can just hardcode i32



Comment at: llvm/test/CodeGen/AMDGPU/bf16.ll:11
+; than simple load/stores.
+
+define void @test_load_store(ptr addrspace(1) %in, ptr addrspace(1) %out) {

Doesn't cover the different f32/f64 conversions?



Comment at: llvm/test/CodeGen/AMDGPU/bf16.ll:2953
+; GFX10-NEXT:s_setpc_b64 s[30:31]
+  %ins.0 = insertvalue { <32 x i32>, bfloat } undef, <32 x i32> %b, 0
+  %ins.1 = insertvalue { <32 x i32>, bfloat } %ins.0 ,bfloat %a, 1

Use

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

2022-12-12 Thread Tom Honermann via Phabricator via cfe-commits 
tahonermann added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5541-5542
+  //
+  // FIXME: Do we need to handle the case that the calculated output is
+  // conflicting with the specified output file or the input file?
+  if (!AtTopLevel && isa(JA) &&

ChuanqiXu wrote:
> dblaikie wrote:
> > Do we do that for `-o` today? (eg: if you try to `-o` and specify the input 
> > file name, such that the output would overwrite the input, what happens? 
> > I'm not sure - but I guess it's OK to do whatever that is for this case too)
> > Do we do that for -o today? (eg: if you try to -o and specify the input 
> > file name, such that the output would overwrite the input, what happens? 
> > I'm not sure - but I guess it's OK to do whatever that is for this case too)
> 
> In this case, the input file will be overwrite and no warning shows. So it 
> looks like we didn't take special treatment here. So I remove the FIXME.
Basing the location of the module output on the presence of `-o` sounds 
confusing to me. Likewise, defaulting to writing next to the input file as 
opposed to the current directory (where a `.o` file is written by default) 
sounds wrong. I think this option should be handled similarly to `-o`; it 
should accept a path and:
  - If an absolute path is provided, write to that location.
  - If a relative path is provided, write to that location relative to the 
current working directory.
Leave it up to the user or build system to ensure that the `.o` and `.pcm` file 
locations coincide if that is what they want. In general, I don't expect 
colocation of `.o` and `.pcm` files to be what is desired.




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

https://reviews.llvm.org/D137058

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

[PATCH] D139045: [HIP] support --offload-arch=native

2022-12-12 Thread Artem Belevich via Phabricator via cfe-commits 
tra accepted this revision.
tra added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Driver/Driver.cpp:3058
   // Collect all offload arch parameters, removing duplicates.
+  const StringRef NativeArchStr = "native";
   std::set GpuArchs;

Nit: It's used only once. May as well compare with "native" directly.



Comment at: clang/lib/Driver/Driver.cpp:3076
+SmallVector GPUs;
+auto Err = TC->detectSystemGPUs(Args, GPUs);
+if (!Err) {

Does it guarantee that a returned list of architectures contains only canonical 
names?


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

https://reviews.llvm.org/D139045

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

[PATCH] D131655: [analyzer] Nullability: Enable analysis of non-inlined nullable objc properties.

2022-12-12 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ added a comment.

F25587055: obama.jpg 


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

https://reviews.llvm.org/D131655

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

[PATCH] D138597: DebugInfo: Add/support new DW_LANG codes for recent C and C++ versions

2022-12-12 Thread David Blaikie via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc73876db4f2d: Reapply "DebugInfo: Add/support new 
DW_LANG codes for recent C and C++… (authored by dblaikie).

Changed prior to commit:
  https://reviews.llvm.org/D138597?vs=480590&id=482273#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138597

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-preprocessed-file.i
  clang/test/CodeGen/debug-info-programming-language.c
  clang/test/CodeGenCXX/debug-info-programming-language.cpp
  clang/test/PCH/debug-info-pch-container-path.c

Index: clang/test/PCH/debug-info-pch-container-path.c
===
--- clang/test/PCH/debug-info-pch-container-path.c
+++ clang/test/PCH/debug-info-pch-container-path.c
@@ -14,7 +14,7 @@
 // RUN: cat %t-container.ll | FileCheck %s
 
 // CHECK: distinct !DICompileUnit(
-// CHECK-SAME:language: DW_LANG_C99,
+// CHECK-SAME:language: DW_LANG_C{{[^,]*}},
 // CHECK-SAME:file: ![[FILE:[0-9]+]],
 // CHECK: ![[FILE]] = !DIFile(
 // CHECK-SAME:filename: "SOURCE/debug-info-limited-struct.h",
Index: clang/test/CodeGenCXX/debug-info-programming-language.cpp
===
--- clang/test/CodeGenCXX/debug-info-programming-language.cpp
+++ clang/test/CodeGenCXX/debug-info-programming-language.cpp
@@ -4,6 +4,12 @@
 // RUN: %clang_cc1 -dwarf-version=3 -emit-llvm -triple %itanium_abi_triple %s -o - \
 // RUN:   -x c++ -std=c++14 -O0 -disable-llvm-passes -debug-info-kind=limited \
 // RUN:   | FileCheck --check-prefix=CHECK-CPP14 %s
+// RUN: %clang_cc1 -dwarf-version=3 -emit-llvm -triple %itanium_abi_triple %s -o - \
+// RUN:   -x c++ -std=c++17 -O0 -disable-llvm-passes -debug-info-kind=limited \
+// RUN:   | FileCheck --check-prefix=CHECK-CPP17 %s
+// RUN: %clang_cc1 -dwarf-version=3 -emit-llvm -triple %itanium_abi_triple %s -o - \
+// RUN:   -x c++ -std=c++20 -O0 -disable-llvm-passes -debug-info-kind=limited \
+// RUN:   | FileCheck --check-prefix=CHECK-CPP20 %s
 // RUN: %clang_cc1 -dwarf-version=3 -gstrict-dwarf -emit-llvm -triple %itanium_abi_triple %s -o - \
 // RUN:   -x c++ -std=c++14 -O0 -disable-llvm-passes -debug-info-kind=limited | FileCheck %s
 // RUN: %clang_cc1 -dwarf-version=5 -gstrict-dwarf -emit-llvm -triple %itanium_abi_triple %s -o - \
@@ -14,5 +20,15 @@
   return 0;
 }
 
+// Update these tests once support for DW_LANG_C_plus_plus_17/20 is added - it's
+// a complicated tradeoff. The language codes are already published/blessed by
+// the DWARF committee, but haven't been released in a published standard yet,
+// so consumers might not be ready for these codes & could regress functionality
+// (because they wouldn't be able to identify that the language was C++). The
+// DWARFv6 language encoding, separating language from language version, would
+// remove this problem/not require new codes for new language versions and make
+// it possible to identify the base language irrespective of the version.
 // CHECK-CPP14: distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14,
+// CHECK-CPP17: distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14,
+// CHECK-CPP20: distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14,
 // CHECK: distinct !DICompileUnit(language: DW_LANG_C_plus_plus,
Index: clang/test/CodeGen/debug-info-programming-language.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-programming-language.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -dwarf-version=3 -emit-llvm -triple %itanium_abi_triple %s -o - \
+// RUN:   -x c -std=c11 -O0 -disable-llvm-passes -debug-info-kind=limited \
+// RUN:   | FileCheck --check-prefix=CHECK-C11 %s
+// RUN: %clang_cc1 -dwarf-version=3 -emit-llvm -triple %itanium_abi_triple %s -o - \
+// RUN:   -x c -std=c17 -O0 -disable-llvm-passes -debug-info-kind=limited \
+// RUN:   | FileCheck --check-prefix=CHECK-C17 %s
+
+// CHECK-C11: !DICompileUnit(language: DW_LANG_C11
+// Update this check once support for DW_LANG_C17 is broadly supported/known in
+// consumers. Maybe we'll skip this and go to the DWARFv6 language+version
+// encoding that avoids the risk of regression when describing a language
+// version newer than what the consumer is aware of.
+// CHECK-C17: !DICompileUnit(language: DW_LANG_C11
+
+void f1(void) { }
Index: clang/test/CodeGen/debug-info-preprocessed-file.i
===
--- clang/test/CodeGen/debug-info-preprocessed-file.i
+++ clang/test/CodeGen/debug-info-preprocessed-file.i
@@ -7,5 +7,5 @@
 # 1 "preprocessed-input.c" 2
 
 // RUN: %clang -g -c -S -emit-llvm -o - %s | FileCheck %s
-// CHECK: !DICompileUnit(language: DW_LANG_C99, file: ![[FILE:[0-9]+]] 
+// CHECK: !DICompileUnit

[clang] c73876d - Reapply "DebugInfo: Add/support new DW_LANG codes for recent C and C++ versions""

2022-12-12 Thread David Blaikie via cfe-commits 

Author: David Blaikie
Date: 2022-12-12T22:36:23Z
New Revision: c73876db4f2dfd2c38d5720f62509e57f23b2059

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

LOG: Reapply "DebugInfo: Add/support new DW_LANG codes for recent C and C++ 
versions""

This may be a breaking change for consumers if they're trying to detect
if code is C or C++, since it'll start using new codes that they may not
be ready to recognize, in which case they may fall back to non-C
handling.

This caused regressions due to PS4 having a different default for C
language version than other targets. Those tests were adapted to be
relaxed about which C version is used.

This reapplies commit 3c312e48f325c1b1ee11404ee6cfa08ee00037b0
Which was reverted by commit 6ab6085c77ef9bcdabf842342f63fba4291791a4.

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

Added: 
clang/test/CodeGen/debug-info-programming-language.c

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGen/debug-info-preprocessed-file.i
clang/test/CodeGenCXX/debug-info-programming-language.cpp
clang/test/PCH/debug-info-pch-container-path.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 2a67f58d22ced..3e370fb17b1d8 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -526,7 +526,8 @@ void CGDebugInfo::CreateCompileUnit() {
 
   // Get absolute path name.
   SourceManager &SM = CGM.getContext().getSourceManager();
-  std::string MainFileName = CGM.getCodeGenOpts().MainFileName;
+  auto &CGO = CGM.getCodeGenOpts();
+  std::string MainFileName = CGO.MainFileName;
   if (MainFileName.empty())
 MainFileName = "";
 
@@ -562,11 +563,11 @@ void CGDebugInfo::CreateCompileUnit() {
   if (LO.CPlusPlus) {
 if (LO.ObjC)
   LangTag = llvm::dwarf::DW_LANG_ObjC_plus_plus;
-else if (LO.CPlusPlus14 && (!CGM.getCodeGenOpts().DebugStrictDwarf ||
-CGM.getCodeGenOpts().DwarfVersion >= 5))
+else if (CGO.DebugStrictDwarf && CGO.DwarfVersion < 5)
+  LangTag = llvm::dwarf::DW_LANG_C_plus_plus;
+else if (LO.CPlusPlus14)
   LangTag = llvm::dwarf::DW_LANG_C_plus_plus_14;
-else if (LO.CPlusPlus11 && (!CGM.getCodeGenOpts().DebugStrictDwarf ||
-CGM.getCodeGenOpts().DwarfVersion >= 5))
+else if (LO.CPlusPlus11)
   LangTag = llvm::dwarf::DW_LANG_C_plus_plus_11;
 else
   LangTag = llvm::dwarf::DW_LANG_C_plus_plus;
@@ -577,6 +578,8 @@ void CGDebugInfo::CreateCompileUnit() {
 LangTag = llvm::dwarf::DW_LANG_OpenCL;
   } else if (LO.RenderScript) {
 LangTag = llvm::dwarf::DW_LANG_GOOGLE_RenderScript;
+  } else if (LO.C11) {
+LangTag = llvm::dwarf::DW_LANG_C11;
   } else if (LO.C99) {
 LangTag = llvm::dwarf::DW_LANG_C99;
   } else {

diff  --git a/clang/test/CodeGen/debug-info-preprocessed-file.i 
b/clang/test/CodeGen/debug-info-preprocessed-file.i
index d231b45d67c2e..23fd26525e3bf 100644
--- a/clang/test/CodeGen/debug-info-preprocessed-file.i
+++ b/clang/test/CodeGen/debug-info-preprocessed-file.i
@@ -7,5 +7,5 @@
 # 1 "preprocessed-input.c" 2
 
 // RUN: %clang -g -c -S -emit-llvm -o - %s | FileCheck %s
-// CHECK: !DICompileUnit(language: DW_LANG_C99, file: ![[FILE:[0-9]+]] 
+// CHECK: !DICompileUnit(language: DW_LANG_C{{.*}}, file: ![[FILE:[0-9]+]]
 // CHECK: ![[FILE]] = !DIFile(filename: "/foo/bar/preprocessed-input.c"

diff  --git a/clang/test/CodeGen/debug-info-programming-language.c 
b/clang/test/CodeGen/debug-info-programming-language.c
new file mode 100644
index 0..f81bab610d51e
--- /dev/null
+++ b/clang/test/CodeGen/debug-info-programming-language.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -dwarf-version=3 -emit-llvm -triple %itanium_abi_triple %s 
-o - \
+// RUN:   -x c -std=c11 -O0 -disable-llvm-passes -debug-info-kind=limited \
+// RUN:   | FileCheck --check-prefix=CHECK-C11 %s
+// RUN: %clang_cc1 -dwarf-version=3 -emit-llvm -triple %itanium_abi_triple %s 
-o - \
+// RUN:   -x c -std=c17 -O0 -disable-llvm-passes -debug-info-kind=limited \
+// RUN:   | FileCheck --check-prefix=CHECK-C17 %s
+
+// CHECK-C11: !DICompileUnit(language: DW_LANG_C11
+// Update this check once support for DW_LANG_C17 is broadly supported/known in
+// consumers. Maybe we'll skip this and go to the DWARFv6 language+version
+// encoding that avoids the risk of regression when describing a language
+// version newer than what the consumer is aware of.
+// CHECK-C17: !DICompileUnit(language: DW_LANG_C11
+
+void f1(void) { }

diff  --git a/clang/test/CodeGenCXX/debug-info-programming-language.cpp 
b/clang/test/CodeGenCXX/debug-info-programming-language.cpp
index 06873a7b5752a..69532661947a8 100644
--- a/clang/test/CodeGenCXX/debug-info-programming-language.

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

2022-12-12 Thread Shafik Yaghmour via Phabricator via cfe-commits 
shafik added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:1594
+  if (VarDecl *VD = dyn_cast(DRE->getDecl())) {
+if (VD->isLocalVarDecl()) {
+  Diag(DRE->getLocation(),

massberg wrote:
> shafik wrote:
> > So if we look at `CheckDefaultArgumentVisitor::VistDeclRefExpr` it has a 
> > comment about having to check ODR use due to [cwg 
> > 2346](https://cplusplus.github.io/CWG/issues/2346.html) and I believe you 
> > should be using that as well.
> > 
> > Which would make your example below pass since the variable is `const` see: 
> > https://godbolt.org/z/78Tojh6q5
> > 
> > It looks like neither gcc nor MSVC implement this DR.
> Thanks! I have extended the check.
> 
> However, the error is still shown for the `const` case in the test.
> When comparing with your function example, I observed that the difference is 
> that cone an `int` and otherwise an `auto` is used.
> I haven't expected a difference, but with `auto` the existing check for local 
> variables as parameters of functions shows an error in case of
> `auto` instead of `int`: https://godbolt.org/z/Kvfvn4aEr (Is this a bug?).
> 
> I have extended the test cases to have cases with `auto` and cases with 
> `int`, but even in the `int` case the code still prints an error.
I am bit surprised that they are considered ODR used in the case you added to 
the test. I am reading the standard and I don't see it but I need to dig some 
more to figure out if I am missing something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139400

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

[PATCH] D139801: [clang-format] Adds 'friend' to QualifierOrder.

2022-12-12 Thread Owen Pan via Phabricator via cfe-commits 
owenpan added a comment.

In D139801#3989812 , 
@HazardyKnusperkeks wrote:

> My idea would be hardcoded `friend` as first entry, regardless of the chosen 
> sequence.
> But others may have a different opinion on that.

It would be confusing with `QAS_Right` and inconsistent with `QAS_Custom` which 
allows e.g. `inline`, `static`, and `volatile` to be placed after the type. IMO 
we should just add `friend` to `QualifierOrder` as this patch currently intends 
to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139801

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

[clang] 6ab01d4 - [analyzer] Nullability: Enable analysis of non-inlined nullable objc properties.

2022-12-12 Thread Artem Dergachev via cfe-commits 

Author: Paul Pelzl
Date: 2022-12-12T14:19:26-08:00
New Revision: 6ab01d4a5cbd46f521de89b167571c0754e6c557

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

LOG: [analyzer] Nullability: Enable analysis of non-inlined nullable objc 
properties.

The NullabilityChecker has a very early policy decision that non-inlined
property accesses will be inferred as returning nonnull, despite nullability
annotations to the contrary. This decision eliminates false positives related
to very common code patterns that look like this:

if (foo.prop) {
[bar doStuffWithNonnull:foo.prop];
}

While this probably represents a correct nil-check, the analyzer can't
determine correctness without gaining visibility into the property
implementation.

Unfortunately, inferring nullable properties as nonnull comes at the cost of
significantly reduced code coverage. My goal here is to enable detection of
many property-related nullability violations without a large increase
in false positives.

The approach is to introduce a heuristic: after accessing the value of
a property, if the analyzer at any time proves that the property value is
nonnull (which would happen in particular due to a nil-check conditional),
then subsequent property accesses on that code path will be *inferred*
as nonnull. This captures the pattern described above, which I believe
to be the dominant source of false positives in real code.

https://reviews.llvm.org/D131655

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
clang/test/Analysis/nullability.mm

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index a016eba29a0d9..da8529f4ea813 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -80,7 +80,7 @@ enum class ErrorKind : int {
 class NullabilityChecker
 : public Checker,
  check::PostCall, check::PostStmt,
- check::PostObjCMessage, check::DeadSymbols,
+ check::PostObjCMessage, check::DeadSymbols, eval::Assume,
  check::Location, check::Event> {
 
 public:
@@ -102,6 +102,8 @@ class NullabilityChecker
   void checkEvent(ImplicitNullDerefEvent Event) const;
   void checkLocation(SVal Location, bool IsLoad, const Stmt *S,
  CheckerContext &C) const;
+  ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
+ bool Assumption) const;
 
   void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
   const char *Sep) const override;
@@ -129,7 +131,7 @@ class NullabilityChecker
   // When set to false no nullability information will be tracked in
   // NullabilityMap. It is possible to catch errors like passing a null pointer
   // to a callee that expects nonnull argument without the information that is
-  // stroed in the NullabilityMap. This is an optimization.
+  // stored in the NullabilityMap. This is an optimization.
   bool NeedTracking = false;
 
 private:
@@ -230,10 +232,41 @@ bool operator==(NullabilityState Lhs, NullabilityState 
Rhs) {
  Lhs.getNullabilitySource() == Rhs.getNullabilitySource();
 }
 
+// For the purpose of tracking historical property accesses, the key for lookup
+// is an object pointer (could be an instance or a class) paired with the 
unique
+// identifier for the property being invoked on that object.
+using ObjectPropPair = std::pair;
+
+// Metadata associated with the return value from a recorded property access.
+struct ConstrainedPropertyVal {
+  // This will reference the conjured return SVal for some call
+  // of the form [object property]
+  DefinedOrUnknownSVal Value;
+
+  // If the SVal has been determined to be nonnull, that is recorded here
+  bool isConstrainedNonnull;
+
+  ConstrainedPropertyVal(DefinedOrUnknownSVal SV)
+  : Value(SV), isConstrainedNonnull(false) {}
+
+  void Profile(llvm::FoldingSetNodeID &ID) const {
+Value.Profile(ID);
+ID.AddInteger(isConstrainedNonnull ? 1 : 0);
+  }
+};
+
+bool operator==(const ConstrainedPropertyVal &Lhs,
+const ConstrainedPropertyVal &Rhs) {
+  return Lhs.Value == Rhs.Value &&
+ Lhs.isConstrainedNonnull == Rhs.isConstrainedNonnull;
+}
+
 } // end anonymous namespace
 
 REGISTER_MAP_WITH_PROGRAMSTATE(NullabilityMap, const MemRegion *,
NullabilityState)
+REGISTER_MAP_WITH_PROGRAMSTATE(PropertyAccessesMap, ObjectPropPair,
+   ConstrainedPropertyVal)
 
 // We say "the nullability type invariant is violated" when a location with a
 // non-null type contains NULL or a function with a

[PATCH] D138675: [flang] Add -ffast-math and -Ofast

2022-12-12 Thread Amy Kwan via Phabricator via cfe-commits 
amyk added a comment.

In D138675#3989997 , @tblah wrote:

> In D138675#3989826 , @amyk wrote:
>
>> Thanks for the follow up patch! I tested the patch locally and also saw the 
>> buildbot results, and it doesn't appear like the follow up patch marked 
>> `powerpc` as unsupported as the error still persists 
>> (https://lab.llvm.org/buildbot/#/builders/21/builds/57850). 
>> I was playing around with the test case locally and what appears to work is 
>> doing something like:
>>
>>   ! UNSUPPORTED: powerpc-registered-target
>
> Thanks 
> https://github.com/llvm/llvm-project/commit/9d86f2dc4f1d2e4e1a991be82384bbdb310f0618

Thanks for following up! The bot is now green 
(https://lab.llvm.org/buildbot/#/builders/21/builds/57857).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138675

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

[PATCH] D102543: [Scudo] Make -fsanitize=scudo use standalone. Migrate tests.

2022-12-12 Thread Mitch Phillips via Phabricator via cfe-commits 
hctim added a comment.
Herald added subscribers: Enna1, abrachet, MaskRay.
Herald added a project: All.

Partially merged (making -fsanitize=scudo use scudo_standalone) merged in 
D138157 . Will probably re-use this in a bit 
to add the dangling scudo tests to scudo_standlaone, but abandoning this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102543

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

[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

2022-12-12 Thread Alan Zhao via Phabricator via cfe-commits 
ayzhao added a comment.

In D129531#3988996 , @ilya-biryukov 
wrote:

> Since the errors only shows up in modular builds, I would look closely for 
> bugs inside `ASTReader`/`ASTWriter`. 
> Also, it seems that `ArrayFiller` is not taken in to account in 
> `computeDependence` and maybe it should be. I am not 100% sure, though: if 
> `ArrayFiller` is only used for non-dependent code, it should not case this 
> bug. It also does not explain the variation between modular and non-modular 
> builds.

These test failures are a little weird because they seem to alternate between 
being able to reliably reproduce vs not being able to reproduce at all.

One thing that I did notice is that the tests fail with `RelWithDebInfo` 
builds, but pass with all of the other builds (`Debug`, `Release`, and 
`MinSizeRel`). Right now though, I'm unable to reproduce the libc++ failure 
even with `RelWithDebInfo`.

Incidentally, I just rebased this patch, and it seems like the dr2xx.cpp test 
 
is also failing in a similar manner (fails on `RelWithDebInfo` but passes on 
other builds).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129531

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

[PATCH] D131655: [analyzer] Nullability: Enable analysis of non-inlined nullable objc properties.

2022-12-12 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ added a comment.

(thanks @usama54321 for having a look and talking to me about this offline!)


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

https://reviews.llvm.org/D131655

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

[PATCH] D131655: [analyzer] Nullability: Enable analysis of non-inlined nullable objc properties.

2022-12-12 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ added a comment.

Guess I'll commit?


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

https://reviews.llvm.org/D131655

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

[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

2022-12-12 Thread Alan Zhao via Phabricator via cfe-commits 
ayzhao updated this revision to Diff 482261.
ayzhao added a comment.

rebase + some compiler warning fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129531

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/ComputeDependence.h
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Sema/Initialization.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ComputeDependence.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/p4.cpp
  clang/test/CodeGen/paren-list-agg-init.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/PCH/cxx_paren_init.cpp
  clang/test/PCH/cxx_paren_init.h
  clang/test/SemaCXX/cxx2a-explicit-bool.cpp
  clang/test/SemaCXX/paren-list-agg-init.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1156,7 +1156,7 @@
 
   Parenthesized initialization of aggregates
   https://wg21.link/p0960r3";>P0960R3
-  No
+  Clang 16
 

 https://wg21.link/p1975r0";>P1975R0
Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -643,6 +643,10 @@
 K = CXCursor_RequiresExpr;
 break;
 
+  case Stmt::CXXParenListInitExprClass:
+K = CXCursor_CXXParenListInitExpr;
+break;
+
   case Stmt::MSDependentExistsStmtClass:
 K = CXCursor_UnexposedStmt;
 break;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2139,6 +2139,7 @@
   void VisitLambdaExpr(const LambdaExpr *E);
   void VisitConceptSpecializationExpr(const ConceptSpecializationExpr *E);
   void VisitRequiresExpr(const RequiresExpr *E);
+  void VisitCXXParenListInitExpr(const CXXParenListInitExpr *E);
   void VisitOMPExecutableDirective(const OMPExecutableDirective *D);
   void VisitOMPLoopBasedDirective(const OMPLoopBasedDirective *D);
   void VisitOMPLoopDirective(const OMPLoopDirective *D);
@@ -3006,6 +3007,9 @@
   for (ParmVarDecl *VD : E->getLocalParameters())
 AddDecl(VD);
 }
+void EnqueueVisitor::VisitCXXParenListInitExpr(const CXXParenListInitExpr *E) {
+  EnqueueChildren(E);
+}
 void EnqueueVisitor::VisitPseudoObjectExpr(const PseudoObjectExpr *E) {
   // Treat the expression like its syntactic form.
   Visit(E->getSyntacticForm());
@@ -5587,6 +5591,8 @@
 return cxstring::createRef("ConceptSpecializationExpr");
   case CXCursor_RequiresExpr:
 return cxstring::createRef("RequiresExpr");
+  case CXCursor_CXXParenListInitExpr:
+return cxstring::createRef("CXXParenListInitExpr");
   case CXCursor_UnexposedStmt:
 return cxstring::createRef("UnexposedStmt");
   case CXCursor_DeclStmt:
Index: clang/test/SemaCXX/paren-list-agg-init.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/paren-list-agg-init.cpp
@@ -0,0 +1,172 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s -fsyntax-only
+// RUN: %clang_cc1 -verify=expected,beforecxx20 -Wc++20-extensions -std=c++20 %s -fsyntax-only
+
+struct A { // expected-note 4{{candidate constructor}}
+  char i;
+  double j;
+};
+
+struct B {
+  A a;
+  int b[20];
+  int &&c; // expected-note {{reference member declared here}}
+};
+
+struct C { // expected-note 5{{candidate constructor}}
+  A a;
+  int b[20];
+};
+
+struct D : public C, public A {
+  int a;
+};
+
+struct E { // expected-note 3{{candidate constructor}}
+  struct F {
+F(int, int);
+  };
+  int a;
+  F f;
+};
+
+int getint(); // expected-note {{declared here}}
+
+struct F {
+  int a;
+  int b = getint(); // expected-note {{non-constexpr function 'getint' cannot be used in a constant expression}}
+};

[PATCH] D138675: [flang] Add -ffast-math and -Ofast

2022-12-12 Thread Tom Eccles via Phabricator via cfe-commits 
tblah added a comment.

In D138675#3989826 , @amyk wrote:

> Thanks for the follow up patch! I tested the patch locally and also saw the 
> buildbot results, and it doesn't appear like the follow up patch marked 
> `powerpc` as unsupported as the error still persists 
> (https://lab.llvm.org/buildbot/#/builders/21/builds/57850). 
> I was playing around with the test case locally and what appears to work is 
> doing something like:
>
>   ! UNSUPPORTED: powerpc-registered-target

Thanks 
https://github.com/llvm/llvm-project/commit/9d86f2dc4f1d2e4e1a991be82384bbdb310f0618


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138675

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

[PATCH] D139881: [clang] Use a StringRef instead of a raw char pointer to store builtin and call information

2022-12-12 Thread serge via Phabricator via cfe-commits 
serge-sans-paille created this revision.
serge-sans-paille added a reviewer: nikic.
Herald added subscribers: steakhal, pmatos, asb, jhenderson, abrachet, ormris, 
martong, phosek, kadircet, arphaman, steven_wu, hiraditya, arichardson, sbc100, 
emaste.
Herald added a reviewer: JDevlieghere.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: jhenderson.
Herald added a reviewer: MaskRay.
Herald added a reviewer: NoQ.
Herald added projects: lld-macho, All.
Herald added a reviewer: lld-macho.
serge-sans-paille requested review of this revision.
Herald added subscribers: cfe-commits, llvm-commits, lldb-commits, StephenFan, 
aheejin.
Herald added projects: clang, LLDB, LLVM, clang-tools-extra.

This avoids recomputing string lentgh that is already known at compile
time.

It has a slight impact on preprocessing / compile time, see

https://llvm-compile-time-tracker.com/compare.php?from=3f36d2d579d8b0e8824d9dd99bfa79f456858f88&to=e49640c507ddc6615b5e503144301c8e41f8f434&stat=instructions:u


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139881

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/include/clang/Basic/Builtins.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Basic/Builtins.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Driver/DriverOptions.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
  clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/cert/PutenvWithAutoChecker.cpp
  clang/lib/StaticAnalyzer/Core/CallDescription.cpp
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp
  clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp
  clang/unittests/StaticAnalyzer/ConflictingEvalCallsTest.cpp
  clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
  clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp
  lld/COFF/DriverUtils.cpp
  lld/ELF/DriverUtils.cpp
  lld/MachO/DriverUtils.cpp
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-server/lldb-gdbserver.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  llvm/include/llvm/ADT/ArrayRef.h
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.cpp
  llvm/lib/Option/OptTable.cpp
  llvm/lib/Option/Option.cpp
  llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
  llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
  llvm/tools/llvm-ifs/llvm-ifs.cpp
  llvm/tools/llvm-lipo/llvm-lipo.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-mt/llvm-mt.cpp
  llvm/tools/llvm-nm/llvm-nm.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-rc/llvm-rc.cpp
  llvm/tools/llvm-readobj/llvm-readobj.cpp
  llvm/tools/llvm-size/llvm-size.cpp
  llvm/tools/llvm-strings/llvm-strings.cpp
  llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -251,8 +251,8 @@
 // Prefix values.
 OS << ", {";
 for (const auto &PrefixKey : Prefix.first)
-  OS << "\"" << PrefixKey << "\" COMMA ";
-OS << "nullptr})\n";
+  OS << "llvm::StringLiteral(\"" << PrefixKey << "\") COMMA ";
+OS << "llvm::StringLiteral(\"\")})\n";
   }
   OS << "#undef COMMA\n";
   OS << "#endif // PREFIX\n\n";
Index: llvm/unittests/Option/OptionParsingTest

[PATCH] D139006: [UpdateTestChecks] Match define for labels

2022-12-12 Thread Nikita Popov via Phabricator via cfe-commits 
nikic added a comment.

In D139006#3988227 , @sebastian-ne 
wrote:

> In D139006#3988215 , @mkazantsev 
> wrote:
>
>> So now every single test needs to be regenerated? It'll create straw diff 
>> from nowhere...
>
> We don’t need to regenerate every test.
> Similar to how we don’t reformat all of LLVM after a change in clang-format, 
> we can just do that when regenerating tests when they have changes anyway.
> So, the churn is that we have extra changes when regenerating a test for a 
> patch.

This kind of change requires generating check lines in a separate commit prior 
to the patch, because functional patches must not contain this kind of noise. 
This kind of change to UTC output is very annoying.

Please revert this patch pending further discussion.

I think there's three options here:

1. The status quo, which is that you must pass `--function-signature` if you 
encounter this issue.
2. What this patch does, which causes massive churn.
3. Something like adding `--function-signature` to UTC_ARGS by default for new 
tests, while not doing so for old ones.

If we are not content with the status quo, then we should do something along 
the lines of 3. Possibly not with `--function-signature` but something like a 
`--version N` flag that defaults to the latest but gets persisted in UTC_ARGS. 
This way we are free to improve UTC without being worried about test churn.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139006

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

[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

2022-12-12 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ added a comment.

Ok so at the conference @ymandel suggested us to use libTooling's Transformers 
API to make our lives easier 
(https://clang.llvm.org/docs/ClangTransformerTutorial.html). I'm fairly certain 
we should at least consider using them, but I haven't looked into it deeply 
enough yet. "Changing the type of a variable" is a problem that ideally should 
be solved only once, because it's a problem that's easy to formulate but 
difficult to "get right". Transformers appear to be a collection of solutions 
to problems of this kind. We should see if they already provide an 
out-of-the-box solution, or if they have bits and pieces of machinery we can 
reuse in our solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139737

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

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

2022-12-12 Thread Shafik Yaghmour via Phabricator via cfe-commits 
shafik added a comment.

In D139713#3989709 , @Pierre-vh wrote:

> In D139713#3989071 , @shafik wrote:
>
>> If I am reading the code correctly it looks like if the call to 
>> `(*I)->isValueDependent()` is true then the temporary will not be created 
>> and therefore we should not be attempting to access the slot.
>>
>> If this is the case then maybe the checking in 
>> `EvaluateWithSubstitution(...)` needs to be more carefully done?
>>
>> I am not familiar with this code but I don't know if you analysis provides a 
>> convincing case the assert should be removed.
>
> Indeed, this only happens when isValueDependent returns true.
>
> I am also not familiar with the code, so I just decided to propose a quick 
> fix to get the discussion started; I certainly don't mind changing the nature 
> of the fix if we agree it should be fixed differently.
> For instance, we could also make the "getParam" call faillible by adding some 
> "tryGetParam" variant that doesn't have the assert, or by passing some 
> optional boolean to indicate it's acceptable to have the key present in the 
> map with a different version.
>
> My initial reasoning was that if the assert can be broken by legitimate code, 
> then it shouldn't be there

It looks like the original commit that added `getTemporary(...)` which was 
4e2698ca9e49e that this invariant held but the later commit which added 
`getParamSlot(...)`  which is f7f2e4261a98b 
 broke the 
invariant.

So I think that looks like an error and any fix should maintain the invariant 
unless strongly motivated reasoning can be put forward to remove it but I am 
not totally sure.

CC @rsmith @ahatanak who are the authors of the two commits mentioned above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139713

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

[clang] 06911ba - [NFC] Cleanup: Replaces BB->getInstList().insert() with I->insertAt().

2022-12-12 Thread Vasileios Porpodas via cfe-commits 

Author: Vasileios Porpodas
Date: 2022-12-12T13:33:05-08:00
New Revision: 06911ba6ea1e552d3bcaed2728c92a9aa6cbf4d2

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

LOG: [NFC] Cleanup: Replaces BB->getInstList().insert() with I->insertAt().

This is part of a series of cleanup patches towards making 
BasicBlock::getInstList() private.

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

Added: 


Modified: 
clang/lib/CodeGen/CGCleanup.cpp
llvm/include/llvm/IR/IRBuilder.h
llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/CodeGen/WinEHPrepare.cpp
llvm/lib/IR/Instruction.cpp
llvm/lib/IR/Instructions.cpp
llvm/lib/Transforms/IPO/IROutliner.cpp
llvm/lib/Transforms/InstCombine/InstCombineInternal.h
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp
llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
llvm/lib/Transforms/Scalar/JumpThreading.cpp
llvm/lib/Transforms/Scalar/LICM.cpp
llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
llvm/lib/Transforms/Scalar/TLSVariableHoist.cpp
llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
llvm/lib/Transforms/Utils/CloneFunction.cpp
llvm/lib/Transforms/Utils/CodeExtractor.cpp
llvm/lib/Transforms/Utils/LibCallsShrinkWrap.cpp
llvm/lib/Transforms/Utils/LowerSwitch.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/lib/Transforms/Utils/UnifyFunctionExitNodes.cpp
llvm/unittests/Analysis/AssumeBundleQueriesTest.cpp
llvm/unittests/Analysis/MemorySSATest.cpp
llvm/unittests/Analysis/ValueTrackingTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCleanup.cpp b/clang/lib/CodeGen/CGCleanup.cpp
index a1ba1a9a50d13..94e25ae8f4476 100644
--- a/clang/lib/CodeGen/CGCleanup.cpp
+++ b/clang/lib/CodeGen/CGCleanup.cpp
@@ -942,7 +942,7 @@ void CodeGenFunction::PopCleanupBlock(bool 
FallthroughIsBranchThrough) {
   // Append the prepared cleanup prologue from above.
   llvm::BasicBlock *NormalExit = Builder.GetInsertBlock();
   for (unsigned I = 0, E = InstsToAppend.size(); I != E; ++I)
-NormalExit->getInstList().push_back(InstsToAppend[I]);
+InstsToAppend[I]->insertAt(NormalExit, NormalExit->end());
 
   // Optimistically hope that any fixups will continue falling through.
   for (unsigned I = FixupDepth, E = EHStack.getNumBranchFixups();

diff  --git a/llvm/include/llvm/IR/IRBuilder.h 
b/llvm/include/llvm/IR/IRBuilder.h
index 592d608fea7bc..6da83e99130af 100644
--- a/llvm/include/llvm/IR/IRBuilder.h
+++ b/llvm/include/llvm/IR/IRBuilder.h
@@ -65,7 +65,8 @@ class IRBuilderDefaultInserter {
   virtual void InsertHelper(Instruction *I, const Twine &Name,
 BasicBlock *BB,
 BasicBlock::iterator InsertPt) const {
-if (BB) BB->getInstList().insert(InsertPt, I);
+if (BB)
+  I->insertAt(BB, InsertPt);
 I->setName(Name);
   }
 };

diff  --git a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h 
b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
index 54d221053e668..81b982a23da26 100644
--- a/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
+++ b/llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
@@ -397,7 +397,7 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner {
 assert(New && !New->getParent() &&
"New instruction already inserted into a basic block!");
 BasicBlock *BB = Old.getParent();
-BB->getInstList().insert(Old.getIterator(), New); // Insert inst
+New->insertAt(BB, Old.getIterator()); // Insert inst
 Worklist.push(New);
 return New;
   }

diff  --git a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h 
b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
index 3b2fe817c04cf..0464474c829ba 100644
--- a/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
@@ -121,8 +121,8 @@ void ReplaceInstWithValue(BasicBlock::InstListType &BIL,
 /// Copies DebugLoc from BI to I, if I doesn't already have a DebugLoc. The
 /// original instruction is deleted and BI is updated to point to the new
 /// instruction.
-void ReplaceInstWithInst(BasicBlock::InstListType &BIL,
- BasicBlock::iterator &BI, Instruction *I);
+void ReplaceInstWithInst(BasicBlock *BB, BasicBlock::iterator &BI,
+ Instruction *I);
 
 /// Replace the instruction specified by From with the instruction specified by
 /// To. Copie

[PATCH] D138877: [NFC] Cleanup: Replaces BB->getInstList().insert() with I->insertAt().

2022-12-12 Thread Vasileios Porpodas 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 rG06911ba6ea1e: [NFC] Cleanup: Replaces 
BB->getInstList().insert() with I->insertAt(). (authored by vporpo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138877

Files:
  clang/lib/CodeGen/CGCleanup.cpp
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/Transforms/InstCombine/InstCombiner.h
  llvm/include/llvm/Transforms/Utils/BasicBlockUtils.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/CodeGen/WinEHPrepare.cpp
  llvm/lib/IR/Instruction.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/Transforms/IPO/IROutliner.cpp
  llvm/lib/Transforms/InstCombine/InstCombineInternal.h
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/lib/Transforms/Instrumentation/ControlHeightReduction.cpp
  llvm/lib/Transforms/Instrumentation/PGOMemOPSizeOpt.cpp
  llvm/lib/Transforms/Scalar/JumpThreading.cpp
  llvm/lib/Transforms/Scalar/LICM.cpp
  llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
  llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
  llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp
  llvm/lib/Transforms/Scalar/TLSVariableHoist.cpp
  llvm/lib/Transforms/Utils/BasicBlockUtils.cpp
  llvm/lib/Transforms/Utils/CloneFunction.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp
  llvm/lib/Transforms/Utils/LibCallsShrinkWrap.cpp
  llvm/lib/Transforms/Utils/LowerSwitch.cpp
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/lib/Transforms/Utils/UnifyFunctionExitNodes.cpp
  llvm/unittests/Analysis/AssumeBundleQueriesTest.cpp
  llvm/unittests/Analysis/MemorySSATest.cpp
  llvm/unittests/Analysis/ValueTrackingTest.cpp

Index: llvm/unittests/Analysis/ValueTrackingTest.cpp
===
--- llvm/unittests/Analysis/ValueTrackingTest.cpp
+++ llvm/unittests/Analysis/ValueTrackingTest.cpp
@@ -1624,7 +1624,7 @@
   EXPECT_EQ(Known.Zero.getZExtValue(), 0u);
 
   BasicBlock *BB = BasicBlock::Create(Context);
-  BB->getInstList().push_back(CI);
+  CI->insertAt(BB, BB->end());
   Known = computeKnownBits(CI, M.getDataLayout(), /* Depth */ 0);
   // There is no parent function so we cannot look up the vscale_range
   // attribute to determine the number of bits.
Index: llvm/unittests/Analysis/MemorySSATest.cpp
===
--- llvm/unittests/Analysis/MemorySSATest.cpp
+++ llvm/unittests/Analysis/MemorySSATest.cpp
@@ -282,7 +282,7 @@
   // - remove from original block
 
   LoadInst *LoadInstClone = cast(LoadInst1->clone());
-  Merge->getInstList().insert(Merge->begin(), LoadInstClone);
+  LoadInstClone->insertAt(Merge, Merge->begin());
   MemoryAccess * NewLoadAccess =
   Updater.createMemoryAccessInBB(LoadInstClone, nullptr,
  LoadInstClone->getParent(),
Index: llvm/unittests/Analysis/AssumeBundleQueriesTest.cpp
===
--- llvm/unittests/Analysis/AssumeBundleQueriesTest.cpp
+++ llvm/unittests/Analysis/AssumeBundleQueriesTest.cpp
@@ -429,7 +429,7 @@
   BasicBlock *BB = BasicBlock::Create(C);
   BB->insertInto(F);
   Instruction *Ret = ReturnInst::Create(C);
-  BB->getInstList().insert(BB->begin(), Ret);
+  Ret->insertAt(BB, BB->begin());
   Function *FnAssume = Intrinsic::getDeclaration(Mod.get(), Intrinsic::assume);
 
   std::vector ShuffledArgs;
Index: llvm/lib/Transforms/Utils/UnifyFunctionExitNodes.cpp
===
--- llvm/lib/Transforms/Utils/UnifyFunctionExitNodes.cpp
+++ llvm/lib/Transforms/Utils/UnifyFunctionExitNodes.cpp
@@ -90,7 +90,7 @@
 // If the function doesn't return void... add a PHI node to the block...
 PN = PHINode::Create(F.getReturnType(), ReturningBlocks.size(),
  "UnifiedRetVal");
-NewRetBlock->getInstList().push_back(PN);
+PN->insertAt(NewRetBlock, NewRetBlock->end());
 ReturnInst::Create(F.getContext(), PN, NewRetBlock);
   }
 
Index: llvm/lib/Transforms/Utils/SimplifyCFG.cpp
===
--- llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1129,7 +1129,7 @@
 NewBonusInst->dropUndefImplyingAttrsAndUnknownMetadata(
 LLVMContext::MD_annotation);
 
-PredBlock->getInstList().insert(PTI->getIterator(), NewBonusInst);
+NewBonusInst->insertAt(PredBlock, PTI->getIterator());
 NewBonusInst->takeName(&BonusInst);
 BonusInst.setName(NewBonusInst->getName() + ".old");
 
@@ -1694,7 +1694,7 @@
 
   // Okay, it is safe to hoist the terminator.
   Instruction *NT = I1->clone();
-  BIParent->getInstList().insert(BI->getIterator(), NT);
+  NT->insertAt(BIParent, BI->getIterator());
   if (!NT->getType()->isVoidTy()) {
 I1

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

2022-12-12 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie added a subscriber: urnathan.
dblaikie added a comment.

(I'd probably still reduce the test down to one example, using `-o` and 
skipping the extra `OUTPUT_PATH` details, only checking the last part of the 
path is as specified (or perhaps checking that it matches the .o file?))

Also, could you consider the questions @urnathan asked in the 
cross-project/flag naming thread about the semantics of this flag - I imagine 
the right behavior is "whatever we do for .o files", but it'd be good to 
check/consider/test them (they may not need automated testing - if the 
behaviour is implemented with the same utilities for .o files as for .pcm files 
then I don't mind just a statement that that's the case and some manual testing 
has been done to verify that all works)?




Comment at: clang/test/Driver/save-std-c++-module-file.cpp:8-9
+//
+// RUN: %clang -std=c++20 %t/Hello.cppm -fmodule-output -o %t/output/Hello.o \
+// RUN:   -### 2>&1 | FileCheck %t/Hello.cppm -DPREFIX=%t 
--check-prefix=CHECK-WITH-OUTPUT
+

ChuanqiXu wrote:
> dblaikie wrote:
> > Not sure I understand the need for two tests here - they both specify an 
> > absolute path to a .o file & CHECK that the absolute path matches the .pcm 
> > output file path, so they don't seem to be testing distinct scenarios?
> The above one doesn't specify the path for the .o file. So in the first test 
> the .pcm file will be in the same directory with the input source file. And 
> in the second command line, the .pcm file will be in the same directory with 
> the .o file.
Ah, maybe just testing the explicit `-o` version is adequate? We aren't doing 
anything interesting here except "whatever the .o path is" - so testing just 
"-o" seems sufficient to test the one path through this code. The pcm code 
doesn't have a "-o" and "implicit -o" codepath, just "do whatever the .o path 
is" so one test seems fine to me?



Comment at: clang/test/Driver/save-std-c++-module-file.cpp:6
+// RUN: %clang -std=c++20 %t/Hello.cppm -fsave-std-c++-module-file -### 2>&1 | 
\
+// RUN:   FileCheck %t/Hello.cppm -DPREFIX=%t
+//

ChuanqiXu wrote:
> dblaikie wrote:
> > Is the prefix needed? I'd expect we'd usually regex match away the actual 
> > directory with `{{.*}}` in tests like this?
> Yeah, generally we would use `{{.*}}`. And `-DPREFIX` is more precise for 
> what we want to test. So I feel it wouldn't be bad.
I think the extra precision probably isn't necessary/sufficiently valuable & we 
could use `{{.*}}` here?


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

https://reviews.llvm.org/D137058

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

[PATCH] D137059: [Driver] [C++20] [Modules] Support -fmodule-output= (2/2)

2022-12-12 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie accepted this revision.
dblaikie added a comment.

In D137059#3989856 , @dblaikie wrote:

> Nathan had a few questions in the cross-project flag naming thread - could 
> you check/reply to those? I think the right answer is probably "do whatever 
> -o does" but would be good to verify that behavior makes sense, maybe 
> explicitly test it in some way if it's significant enough/requires any work 
> to support getting that behavior? (if it falls out naturally from some 
> existing file IO handling we have, I'm less worried about testing it 
> separately)

Oh, I guess those questions ^ are really more applicable to the previous patch 
in the series, that introduces writing the file out anyway - the ability to 
specify the name of the file probably has no impact/shouldn't have any impact 
on the answers to the questions, so this patch looks good - please consider 
those issues in the other patch.


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

https://reviews.llvm.org/D137059

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

[PATCH] D137059: [Driver] [C++20] [Modules] Support -fmodule-output= (2/2)

2022-12-12 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie added a comment.

Nathan had a few questions in the cross-project flag naming thread - could you 
check/reply to those? I think the right answer is probably "do whatever -o 
does" but would be good to verify that behavior makes sense, maybe explicitly 
test it in some way if it's significant enough/requires any work to support 
getting that behavior? (if it falls out naturally from some existing file IO 
handling we have, I'm less worried about testing it separately)


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

https://reviews.llvm.org/D137059

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

[PATCH] D138877: [NFC] Cleanup: Replaces BB->getInstList().insert() with I->insertAt().

2022-12-12 Thread Vasileios Porpodas via Phabricator via cfe-commits 
vporpo added a comment.
Herald added a subscriber: kmitropoulou.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138877

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

[PATCH] D139045: [HIP] support --offload-arch=native

2022-12-12 Thread Yaxun Liu via Phabricator via cfe-commits 
yaxunl updated this revision to Diff 482240.
yaxunl added a comment.

fix error handling


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

https://reviews.llvm.org/D139045

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h


Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -107,6 +107,9 @@
   llvm::Error getSystemGPUArch(const llvm::opt::ArgList &Args,
std::string &GPUArch) const;
 
+  llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
+   SmallVector &GPUArchs) const;
+
 protected:
   /// Check and diagnose invalid target ID specified by -mcpu.
   virtual void checkTargetID(const llvm::opt::ArgList &DriverArgs) const;
@@ -126,8 +129,6 @@
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList &DriverArgs) const;
 
-  llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
-   SmallVector &GPUArchs) const;
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3055,6 +3055,7 @@
   }
 
   // Collect all offload arch parameters, removing duplicates.
+  const StringRef NativeArchStr = "native";
   std::set GpuArchs;
   bool Error = false;
   for (Arg *A : Args) {
@@ -3067,6 +3068,17 @@
   if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
   ArchStr == "all") {
 GpuArchs.clear();
+  } else if (ArchStr == NativeArchStr &&
+ ToolChains.front()->getTriple().isAMDGPU()) {
+auto *TC = static_cast(
+ToolChains.front());
+SmallVector GPUs;
+auto Err = TC->detectSystemGPUs(Args, GPUs);
+if (!Err) {
+  for (auto GPU : GPUs)
+GpuArchs.insert(Args.MakeArgString(GPU));
+} else
+  llvm::consumeError(std::move(Err));
   } else {
 ArchStr = getCanonicalOffloadArch(ArchStr);
 if (ArchStr.empty()) {


Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -107,6 +107,9 @@
   llvm::Error getSystemGPUArch(const llvm::opt::ArgList &Args,
std::string &GPUArch) const;
 
+  llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
+   SmallVector &GPUArchs) const;
+
 protected:
   /// Check and diagnose invalid target ID specified by -mcpu.
   virtual void checkTargetID(const llvm::opt::ArgList &DriverArgs) const;
@@ -126,8 +129,6 @@
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList &DriverArgs) const;
 
-  llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
-   SmallVector &GPUArchs) const;
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3055,6 +3055,7 @@
   }
 
   // Collect all offload arch parameters, removing duplicates.
+  const StringRef NativeArchStr = "native";
   std::set GpuArchs;
   bool Error = false;
   for (Arg *A : Args) {
@@ -3067,6 +3068,17 @@
   if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
   ArchStr == "all") {
 GpuArchs.clear();
+  } else if (ArchStr == NativeArchStr &&
+ ToolChains.front()->getTriple().isAMDGPU()) {
+auto *TC = static_cast(
+ToolChains.front());
+SmallVector GPUs;
+auto Err = TC->detectSystemGPUs(Args, GPUs);
+if (!Err) {
+  for (auto GPU : GPUs)
+GpuArchs.insert(Args.MakeArgString(GPU));
+} else
+  llvm::consumeError(std::move(Err));
   } else {
 ArchStr = getCanonicalOffloadArch(ArchStr);
 if (ArchStr.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

2022-12-12 Thread Louis Dionne via Phabricator via cfe-commits 
ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

This LGTM.




Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2084-2089
+  // On Apple platforms, C and C++ Standard Library headers are not provided
+  // with the base system. Instead, they are provided in various SDKs for the
+  // different Apple platforms. Clang needs to know where that SDK lives, and
+  // there are a couple ways this can be achieved:
+  //
+  // (1) If `-isysroot ` is passed explicitly, use that.

Instead of this:

> On Apple platforms, C and C++ Standard Library headers are not provided with 
> the base system.

I would say this, which is less specific to headers:

> On Apple platforms, standard headers and libraries are not provided with the 
> base system (e.g. in `/usr/{include,lib}`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136315

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

[PATCH] D138675: [flang] Add -ffast-math and -Ofast

2022-12-12 Thread Amy Kwan via Phabricator via cfe-commits 
amyk added a comment.

In D138675#3989634 , @tblah wrote:

> In D138675#3989403 , @amyk wrote:
>
>> Hi,
>>
>> The ppc64le-flang-rhel-clang  
>> bot is also experiencing some failures with the `Driver/fast_math.f90` test 
>> case. In particular, `crtfastmath.o` is not found at all for `CHECK-CRT` as 
>> we can see in the failure details 
>> .
>>
>>   llvm-project/flang/test/Driver/fast_math.f90:64:19: error: CHECK-CRT-SAME: 
>> expected string not found in input
>>   ! CHECK-CRT-SAME: crtfastmath.o
>>
>> The output for the `CHECK-CRT` line on the bot looks like the following:
>>
>>   
>> "/home/buildbots/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/build/bin/flang-new"
>>  "-fc1" "-triple" "powerpc64le-unknown-linux-gnu" "-emit-obj" 
>> "-mrelocation-model" "pic" "-pic-level" "2" "-pic-is-pie" "-ffast-math" 
>> "-target-cpu" "ppc64le" "-o" "/tmp/lit-tmp-x_okwzug/fast_math-e9ab49.o" "-x" 
>> "f95-cpp-input" 
>> "/home/buildbots/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Driver/fast_math.f90"
>>  
>>   
>>   "/usr/bin/ld" "-pie" "--hash-style=gnu" "--eh-frame-hdr" "-m" "elf64lppc" 
>> "-dynamic-linker" "/lib64/ld64.so.2" "-o" 
>> "/home/buildbots/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/build/tools/flang/test/Driver/Output/fast_math.f90.tmp"
>>  "/usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64/Scrt1.o" 
>> "/usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64/crti.o" 
>> "/usr/lib/gcc/ppc64le-redhat-linux/8/crtbeginS.o" 
>> "-L/usr/lib/gcc/ppc64le-redhat-linux/8" 
>> "-L/usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64" "-L/lib/../lib64" 
>> "-L/usr/lib/../lib64" "-L/lib" "-L/usr/lib" 
>> "/tmp/lit-tmp-x_okwzug/fast_math-e9ab49.o" "-lFortran_main" 
>> "-lFortranRuntime" "-lFortranDecimal" "-lm" "-lgcc" "--as-needed" "-lgcc_s" 
>> "--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
>> "/usr/lib/gcc/ppc64le-redhat-linux/8/crtendS.o" 
>> "/usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64/crtn.o"   
>>  
>>
>> Would it be possible to follow up with a fix for this test case?
>
> Thanks for letting me know. I thought I had already disabled that test on 
> powerpc (in 
> https://github.com/llvm/llvm-project/commit/20cd3153f3775fcdc1eeeb54062849eead51e24a)
>  but apparently it didn't work. I don't have a powerpc system available to 
> test this. I have pushed a second attempt at 
> https://github.com/llvm/llvm-project/commit/6442b4da4e7018e8f264965768b9e4fdee393c8f.
>  Please let me know if that works.

Thanks for the follow up patch! I tested the patch locally and also saw the 
buildbot results, and it doesn't appear like the follow up patch marked 
`powerpc` as unsupported as the error still persists 
(https://lab.llvm.org/buildbot/#/builders/21/builds/57850). 
I was playing around with the test case locally and what appears to work is 
doing something like:

  ! UNSUPPORTED: powerpc-registered-target


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138675

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

[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

2022-12-12 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin added a comment.

Hey. Thanks a lot for working on this.
I did a first pass, looking at mostly style issues, looking at conformance will 
probably take me a lot more time, but i think this looks pretty good overall




Comment at: clang/include/clang/AST/DeclCXX.h:1911
   setRangeEnd(EndLocation);
-setIsCopyDeductionCandidate(false);
+setDeductionCandidateKind(DeductionCandidateKind::Normal);
   }

I'm wondering if the constructor should take a `DeductionCandidateKind` 
defaulted to normal here. All the places where it's set seem to be immediately 
after construction.



Comment at: clang/include/clang/AST/DeclCXX.h:1953-1959
   bool isCopyDeductionCandidate() const {
-return FunctionDeclBits.IsCopyDeductionCandidate;
+return getDeductionCandidateKind() == DeductionCandidateKind::Copy;
+  }
+
+  bool isAggregateDeductionCandidate() const {
+return getDeductionCandidateKind() == DeductionCandidateKind::Aggregate;
   }

I'm not sure how useful these things are, isAggregateDeductionCandidate is only 
used once



Comment at: clang/lib/Sema/SemaInit.cpp:39
 #include "llvm/Support/raw_ostream.h"
+#include 
 

This does not seems used



Comment at: clang/lib/Sema/SemaInit.cpp:312
   NoInitExpr *DummyExpr = nullptr;
+  SmallVector *AggrDeductionCandidateParamTypes = nullptr;
 

I think using optional here would make more sense, I guess that's why you 
included it.
Should it be SmallVectorImpl, such that only the caller has to bake in a size?



Comment at: clang/lib/Sema/SemaInit.cpp:1098-1099
 maxElements = T->castAs()->getNumElements();
+  else if (T->isDependentType())
+maxElements = 1;
   else

Can you had a comment about that case? I'm not sure i understand what scenario 
we are handling



Comment at: clang/lib/Sema/SemaInit.cpp:2167-2168
   // If we have any base classes, they are initialized prior to the fields.
-  for (auto &Base : Bases) {
+  for (auto I = Bases.begin(), E = Bases.end(); I != E; ++I) {
+auto &Base = *I;
 Expr *Init = Index < IList->getNumInits() ? IList->getInit(Index) : 
nullptr;

Maybe using `enumerate` + a range for loop here would be cleaner?



Comment at: clang/lib/Sema/SemaOverload.cpp:7220
   MethodTmpl, ExplicitTemplateArgs, Args, Specialization, Info,
-  PartialOverloading, [&](ArrayRef ParamTypes) {
+  PartialOverloading, false, [&](ArrayRef ParamTypes) {
 return CheckNonDependentConversions(




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139837

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

[PATCH] D139801: [clang-format] Adds 'friend' to QualifierOrder.

2022-12-12 Thread Björn Schäpers via Phabricator via cfe-commits 
HazardyKnusperkeks added a comment.

In D139801#3989284 , @red1bluelost 
wrote:

> In D139801#3988790 , 
> @HazardyKnusperkeks wrote:
>
>> If only the first, I'd say we do that without adding this option.
>
> I'll work on doing this without adding the option. (Just reread and noticed 
> you had answered what I asked in my previous comment.)

It should be part of the qualifieralignmentfixer, we don't want to move the 
tokens when someone doesn't have it turned on. My idea would be hardcoded 
`friend` as first entry, regardless of the chosen sequence.
But others may have a different opinion on that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139801

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

[PATCH] D139045: [HIP] support --offload-arch=native

2022-12-12 Thread Yaxun Liu via Phabricator via cfe-commits 
yaxunl updated this revision to Diff 482234.
yaxunl retitled this revision from "[HIP] use detected GPU in --offload-arch" 
to "[HIP] support --offload-arch=native".
yaxunl edited the summary of this revision.
yaxunl added a comment.

use detected GPU when --offload-arch=native is specified based on RFC discussion


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

https://reviews.llvm.org/D139045

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h


Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -107,6 +107,9 @@
   llvm::Error getSystemGPUArch(const llvm::opt::ArgList &Args,
std::string &GPUArch) const;
 
+  llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
+   SmallVector &GPUArchs) const;
+
 protected:
   /// Check and diagnose invalid target ID specified by -mcpu.
   virtual void checkTargetID(const llvm::opt::ArgList &DriverArgs) const;
@@ -126,8 +129,6 @@
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList &DriverArgs) const;
 
-  llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
-   SmallVector &GPUArchs) const;
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3055,6 +3055,7 @@
   }
 
   // Collect all offload arch parameters, removing duplicates.
+  const StringRef NativeArchStr = "native";
   std::set GpuArchs;
   bool Error = false;
   for (Arg *A : Args) {
@@ -3067,6 +3068,15 @@
   if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
   ArchStr == "all") {
 GpuArchs.clear();
+  } else if (ArchStr == NativeArchStr &&
+ ToolChains.front()->getTriple().isAMDGPU()) {
+auto *TC = static_cast(
+ToolChains.front());
+SmallVector GPUs;
+if (!TC->detectSystemGPUs(Args, GPUs)) {
+  for (auto GPU : GPUs)
+GpuArchs.insert(Args.MakeArgString(GPU));
+}
   } else {
 ArchStr = getCanonicalOffloadArch(ArchStr);
 if (ArchStr.empty()) {


Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -107,6 +107,9 @@
   llvm::Error getSystemGPUArch(const llvm::opt::ArgList &Args,
std::string &GPUArch) const;
 
+  llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
+   SmallVector &GPUArchs) const;
+
 protected:
   /// Check and diagnose invalid target ID specified by -mcpu.
   virtual void checkTargetID(const llvm::opt::ArgList &DriverArgs) const;
@@ -126,8 +129,6 @@
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList &DriverArgs) const;
 
-  llvm::Error detectSystemGPUs(const llvm::opt::ArgList &Args,
-   SmallVector &GPUArchs) const;
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3055,6 +3055,7 @@
   }
 
   // Collect all offload arch parameters, removing duplicates.
+  const StringRef NativeArchStr = "native";
   std::set GpuArchs;
   bool Error = false;
   for (Arg *A : Args) {
@@ -3067,6 +3068,15 @@
   if (A->getOption().matches(options::OPT_no_offload_arch_EQ) &&
   ArchStr == "all") {
 GpuArchs.clear();
+  } else if (ArchStr == NativeArchStr &&
+ ToolChains.front()->getTriple().isAMDGPU()) {
+auto *TC = static_cast(
+ToolChains.front());
+SmallVector GPUs;
+if (!TC->detectSystemGPUs(Args, GPUs)) {
+  for (auto GPU : GPUs)
+GpuArchs.insert(Args.MakeArgString(GPU));
+}
   } else {
 ArchStr = getCanonicalOffloadArch(ArchStr);
 if (ArchStr.empty()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139720: [clang][PPC] Checking Unknown Values Passed to -mcpu

2022-12-12 Thread Jamie Schmeiser via Phabricator via cfe-commits 
jamieschmeiser accepted this revision.
jamieschmeiser added a comment.
This revision is now accepted and ready to land.

LGTM

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139720

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

[clang-tools-extra] c187182 - [clangd] Add a missing header guard for InsertionPoint.h

2022-12-12 Thread Haojian Wu via cfe-commits 

Author: Haojian Wu
Date: 2022-12-12T21:25:29+01:00
New Revision: c187182429fb3c8af09827b816c1cb8976bc93e7

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

LOG: [clangd] Add a missing header guard for InsertionPoint.h

Added: 


Modified: 
clang-tools-extra/clangd/refactor/InsertionPoint.h

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/InsertionPoint.h 
b/clang-tools-extra/clangd/refactor/InsertionPoint.h
index eee158b77e1f6..401a384928e11 100644
--- a/clang-tools-extra/clangd/refactor/InsertionPoint.h
+++ b/clang-tools-extra/clangd/refactor/InsertionPoint.h
@@ -6,6 +6,9 @@
 //
 
//===--===//
 
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_INSERTIONPOINT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_INSERTIONPOINT_H
+
 #include "clang/AST/DeclCXX.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Tooling/Core/Replacement.h"
@@ -51,3 +54,5 @@ llvm::Expected 
insertDecl(llvm::StringRef Code,
 
 } // namespace clangd
 } // namespace clang
+
+#endif



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

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

2022-12-12 Thread Pierre van Houtryve via Phabricator via cfe-commits 
Pierre-vh added a comment.

In D139713#3989071 , @shafik wrote:

> If I am reading the code correctly it looks like if the call to 
> `(*I)->isValueDependent()` is true then the temporary will not be created and 
> therefore we should not be attempting to access the slot.
>
> If this is the case then maybe the checking in 
> `EvaluateWithSubstitution(...)` needs to be more carefully done?
>
> I am not familiar with this code but I don't know if you analysis provides a 
> convincing case the assert should be removed.

Indeed, this only happens when isValueDependent returns true.

I am also not familiar with the code, so I just decided to propose a quick fix 
to get the discussion started; I certainly don't mind changing the nature of 
the fix if we agree it should be fixed differently.
For instance, we could also make the "getParam" call faillible by adding some 
"tryGetParam" variant that doesn't have the assert, or by passing some optional 
boolean to indicate it's acceptable to have the key present in the map with a 
different version.

My initial reasoning was that if the assert can be broken by legitimate code, 
then it shouldn't be there


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139713

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

[PATCH] D139720: [clang][PPC] Checking Unknown Values Passed to -mcpu

2022-12-12 Thread Qiongsi Wu via Phabricator via cfe-commits 
qiongsiwu1 added a comment.

In D139720#3989651 , @jamieschmeiser 
wrote:

> I'm fine with handling the return for common as a separate change, if 
> necessary.

Sounds good! In this case I will fix it in a different patch once we decide 
what exactly to do for the `-mcpu=common` case.

> Is the error produced now because it passes back the incorrect option rather 
> than quietly changing it to something appropriate as it did before?

Yes precisely. When a `TargetInfo` is created, we have this 

 code that checks the validity of the CPU string. Currently, we silently change 
anything weird to something reasonable and use it to create the `TargetInfo`. 
This patch removes that filtering and pass on the incorrect CPU string so we 
can catch the error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139720

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

[PATCH] D139720: [clang][PPC] Checking Unknown Values Passed to -mcpu

2022-12-12 Thread Jamie Schmeiser via Phabricator via cfe-commits 
jamieschmeiser added a comment.

I'm fine with handling the return for common as a separate change, if necessary.
Is the error produced now because it passes back the incorrect option rather 
than quietly changing it to something appropriate as it did before?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139720

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

[PATCH] D138675: [flang] Add -ffast-math and -Ofast

2022-12-12 Thread Tom Eccles via Phabricator via cfe-commits 
tblah added a comment.

In D138675#3989403 , @amyk wrote:

> Hi,
>
> The ppc64le-flang-rhel-clang  
> bot is also experiencing some failures with the `Driver/fast_math.f90` test 
> case. In particular, `crtfastmath.o` is not found at all for `CHECK-CRT` as 
> we can see in the failure details 
> .
>
>   llvm-project/flang/test/Driver/fast_math.f90:64:19: error: CHECK-CRT-SAME: 
> expected string not found in input
>   ! CHECK-CRT-SAME: crtfastmath.o
>
> The output for the `CHECK-CRT` line on the bot looks like the following:
>
>   
> "/home/buildbots/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/build/bin/flang-new"
>  "-fc1" "-triple" "powerpc64le-unknown-linux-gnu" "-emit-obj" 
> "-mrelocation-model" "pic" "-pic-level" "2" "-pic-is-pie" "-ffast-math" 
> "-target-cpu" "ppc64le" "-o" "/tmp/lit-tmp-x_okwzug/fast_math-e9ab49.o" "-x" 
> "f95-cpp-input" 
> "/home/buildbots/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Driver/fast_math.f90"
>  
>   
>   "/usr/bin/ld" "-pie" "--hash-style=gnu" "--eh-frame-hdr" "-m" "elf64lppc" 
> "-dynamic-linker" "/lib64/ld64.so.2" "-o" 
> "/home/buildbots/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/build/tools/flang/test/Driver/Output/fast_math.f90.tmp"
>  "/usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64/Scrt1.o" 
> "/usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64/crti.o" 
> "/usr/lib/gcc/ppc64le-redhat-linux/8/crtbeginS.o" 
> "-L/usr/lib/gcc/ppc64le-redhat-linux/8" 
> "-L/usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64" "-L/lib/../lib64" 
> "-L/usr/lib/../lib64" "-L/lib" "-L/usr/lib" 
> "/tmp/lit-tmp-x_okwzug/fast_math-e9ab49.o" "-lFortran_main" 
> "-lFortranRuntime" "-lFortranDecimal" "-lm" "-lgcc" "--as-needed" "-lgcc_s" 
> "--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
> "/usr/lib/gcc/ppc64le-redhat-linux/8/crtendS.o" 
> "/usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64/crtn.o"
> 
>
> Would it be possible to follow up with a fix for this test case?

Thanks for letting me know. I thought I had already disabled that test on 
powerpc (in 
https://github.com/llvm/llvm-project/commit/20cd3153f3775fcdc1eeeb54062849eead51e24a)
 but apparently it didn't work. I don't have a powerpc system available to test 
this. I have pushed a second attempt at 
https://github.com/llvm/llvm-project/commit/6442b4da4e7018e8f264965768b9e4fdee393c8f.
 Please let me know if that works.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138675

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

[PATCH] D138210: [clang] Require parameter pack to be last argument in concepts.

2022-12-12 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie added a comment.

In D138210#3989385 , @luken-google 
wrote:

> committed at afd800fc5679ccfe6f32b097586c658628500867 
> 

For future reference it's handy to include the "Differential Revision: 
https://reviews.llvm.org/D"; (`arc` will create this for you, if you use 
that) in the commit message both so folks looking at the commit can find the 
review/know that it was reviewed on Phab, and that way Phab will auto-close the 
review and include a link to the commit.

Also I think we're still using the "PR" prefix predominantly for bug 
references, rather than the "GH" suffix you've used here (handy for consistency 
- easier to search up the bugs if the prefix is more consistent).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138210

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

[PATCH] D139868: [clang][dataflow] Change the diagnoser API to receive a correctly typed lattice element

2022-12-12 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits 
ymandel accepted this revision.
ymandel added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:134
+PostVisitCFG =
+[=](ASTContext &Context, const CFGElement &Element,
+const TransferStateForDiagnostics

nit: I think capturing by move is a bit better, since Arg is passed by copy to 
`withPostVisitCFG`: `[Arg = std::move(Arg)]`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139868

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

[clang] 11b9c79 - [Clang] Try to fix test on Windows (NFC)

2022-12-12 Thread Nikita Popov via cfe-commits 

Author: Nikita Popov
Date: 2022-12-12T20:36:35+01:00
New Revision: 11b9c7943bad1915e3ba096b597af3d050048d53

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

LOG: [Clang] Try to fix test on Windows (NFC)

Try to fix failure reported in
https://reviews.llvm.org/rG9466b49171dc#1154213 by making the
match more specific, as there are multiple dbg.declares that
could be matched.

Added: 


Modified: 
clang/test/CodeGenObjC/2010-02-09-DbgSelf.m

Removed: 




diff  --git a/clang/test/CodeGenObjC/2010-02-09-DbgSelf.m 
b/clang/test/CodeGenObjC/2010-02-09-DbgSelf.m
index df2f8bec20f05..ad8b29a85ea14 100644
--- a/clang/test/CodeGenObjC/2010-02-09-DbgSelf.m
+++ b/clang/test/CodeGenObjC/2010-02-09-DbgSelf.m
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -x objective-c -emit-llvm -debug-info-kind=limited < %s | 
FileCheck %s
 // Test to check that "self" argument is assigned a location.
-// CHECK: call void @llvm.dbg.declare(metadata ptr %{{[^,]+}}, metadata 
[[SELF:![0-9]*]], metadata !{{.*}})
+// CHECK: call void @llvm.dbg.declare(metadata ptr %self.addr, metadata 
[[SELF:![0-9]*]], metadata !{{.*}})
 // CHECK: [[SELF]] = !DILocalVariable(name: "self", arg: 1,
 
 @interface Foo 



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

[PATCH] D139868: [clang][dataflow] Change the diagnoser API to receive a correctly typed lattice element Previously, the diagnoser could only receive the Environment at a given program point. Now, it

2022-12-12 Thread Dani Ferreira Franco Moura via Phabricator via cfe-commits 
merrymeerkat created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
merrymeerkat requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

...lattice element.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139868

Files:
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1314,7 +1314,8 @@
 [&Diagnostics,
  Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
 ASTContext &Ctx, const CFGElement &Elt,
-const TypeErasedDataflowAnalysisState &State) mutable {
+const TransferStateForDiagnostics
+&State) mutable {
   auto EltDiagnostics =
   Diagnoser.diagnose(Ctx, &Elt, State.Env);
   llvm::move(EltDiagnostics, std::back_inserter(Diagnostics));
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -30,6 +30,7 @@
 #include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Serialization/PCHContainerOperations.h"
@@ -116,11 +117,26 @@
 SetupTest = std::move(Arg);
 return std::move(*this);
   }
+  AnalysisInputs &&withPostVisitCFG(
+  std::function &)>
+  Arg) && {
+PostVisitCFG = std::move(Arg);
+return std::move(*this);
+  }
+
   AnalysisInputs &&
   withPostVisitCFG(std::function
Arg) && {
-PostVisitCFG = std::move(Arg);
+PostVisitCFG =
+[=](ASTContext &Context, const CFGElement &Element,
+const TransferStateForDiagnostics
+&State) {
+  Arg(Context, Element,
+  TypeErasedDataflowAnalysisState({State.Lattice}, State.Env));
+};
 return std::move(*this);
   }
   AnalysisInputs &&withASTBuildArgs(ArrayRef Arg) && {
@@ -148,8 +164,9 @@
   std::function SetupTest = nullptr;
   /// Optional. If provided, this function is applied on each CFG element after
   /// the analysis has been run.
-  std::function
+  std::function &)>
   PostVisitCFG = nullptr;
 
   /// Optional. Options for building the AST context.
@@ -226,11 +243,15 @@
  const TypeErasedDataflowAnalysisState &)>
   PostVisitCFGClosure = nullptr;
   if (AI.PostVisitCFG) {
-PostVisitCFGClosure =
-[&AI, &Context](const CFGElement &Element,
-const TypeErasedDataflowAnalysisState &State) {
-  AI.PostVisitCFG(Context, Element, State);
-};
+PostVisitCFGClosure = [&AI, &Context](
+  const CFGElement &Element,
+  const TypeErasedDataflowAnalysisState &State) {
+  AI.PostVisitCFG(Context, Element,
+  TransferStateForDiagnostics(
+  llvm::any_cast(
+  State.Lattice.Value),
+  State.Env));
+};
   }
 
   // Additional test setup.
@@ -326,28 +347,28 @@
   // Save the states computed for program points immediately following annotated
   // statements. The saved states are keyed by the content of the annotation.
   llvm::StringMap AnnotationStates;
-  auto PostVisitCFG = [&StmtToAnnotations, &AnnotationStates,
-   PrevPostVisitCFG = std::move(AI.PostVisitCFG)](
-  ASTContext &Ctx, const CFGElement &Elt,
-  const TypeErasedDataflowAnalysisState &State) {
-if (PrevPostVisitCFG) {
-  PrevPostVisitCFG(Ctx, Elt, State);
-}
-// FIXME: Extend retrieval of state for non statement constructs.
-auto Stmt = Elt.getAs();
-if (!Stmt)
-  return;
-auto It = StmtToAnnotations.find(Stmt->getStmt());
-if (It == StmtToAnnotations.end())
-  return;
-auto *Lattice =
-llvm::any_cast(&State.Lattice.Value);
-auto [_, InsertSuccess] =
-AnnotationStates.insert({It->second, StateT{*Lattice, State.Env}});
-(void)_;
-(void)InsertSuccess;
-assert(InsertSucce

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

2022-12-12 Thread Paul Robinson 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 rG7793e676514b: [ZOS] Convert tests to check 
'target={{.*}}-zos{{.*}}' (authored by probinson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139444

Files:
  clang/test/Analysis/cfref_PR2519.c
  clang/test/CodeGen/cfstring2.c
  clang/test/Driver/as-version.s
  clang/test/Import/forward-declared-objc-class/test.m
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Import/objc-autoreleasepool/test.m
  clang/test/Import/objc-definitions-in-expression/test.m
  clang/test/Import/objc-method/test.m
  clang/test/Import/objc-param-decl/test.m
  clang/test/Import/objc-try-catch/test.m
  clang/test/Modules/DebugInfoNamespace.cpp
  clang/test/Modules/DebugInfoTransitiveImport.m
  clang/test/Modules/ExtDebugInfo.cpp
  clang/test/Modules/ExtDebugInfo.m
  clang/test/Modules/ModuleDebugInfo.cpp
  clang/test/Modules/ModuleDebugInfo.m
  clang/test/Modules/ModuleDebugInfoDwoId.cpp
  clang/test/Modules/ModuleModuleDebugInfo.cpp
  clang/test/Modules/autolink.m
  clang/test/Modules/autolinkTBD.m
  clang/test/Modules/builtins.m
  clang/test/Modules/clang_module_file_info.m
  clang/test/Modules/cxx-irgen.cpp
  clang/test/Modules/debug-info-moduleimport-in-module.m
  clang/test/Modules/debug-info-moduleimport.m
  clang/test/Modules/direct-module-import.m
  clang/test/Modules/merge-anon-record-definition-in-objc.m
  clang/test/Modules/merge-extension-ivars.m
  clang/test/Modules/merge-objc-interface-visibility.m
  clang/test/Modules/merge-objc-interface.m
  clang/test/Modules/merge-record-definition-nonmodular.m
  clang/test/Modules/merge-record-definition-visibility.m
  clang/test/Modules/merge-record-definition.m
  clang/test/Modules/module-debuginfo-prefix.m
  clang/test/Modules/module-file-home-is-cwd.m
  clang/test/Modules/module_file_info.m
  clang/test/Modules/objc-initializer.m
  clang/test/Modules/pch-used.m
  clang/test/Modules/redecl-ivars.m
  clang/test/Modules/use-exportas-for-link.m
  clang/test/PCH/externally-retained.m
  clang/test/PCH/irgen-rdar13114142.mm
  clang/test/PCH/objc_container.m
  clang/test/PCH/objc_literals.m
  clang/test/PCH/objc_literals.mm
  clang/test/PCH/objcxx-ivar-class.mm
  clang/test/PCH/pending-ids.m
  llvm/test/MC/AsmParser/debug-no-source.s
  llvm/test/Support/encoding.ll
  llvm/test/tools/llvm-mc/no_warnings.test
  llvm/utils/lit/lit/llvm/config.py

Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -88,6 +88,8 @@
 features.add('system-aix')
 elif platform.system() == 'SunOS':
 features.add('system-solaris')
+elif platform.system() == 'OS/390':
+features.add('system-zos')
 
 # Native compilation: host arch == default triple arch
 # Both of these values should probably be in every site config (e.g. as
Index: llvm/test/tools/llvm-mc/no_warnings.test
===
--- llvm/test/tools/llvm-mc/no_warnings.test
+++ llvm/test/tools/llvm-mc/no_warnings.test
@@ -1,4 +1,4 @@
-# UNSUPPORTED: -zos
+# UNSUPPORTED: target={{.*}}-zos{{.*}}
 # RUN: llvm-mc --no-warn %s 2>&1 | FileCheck %s
 
 # CHECK-NOT: warning:
Index: llvm/test/Support/encoding.ll
===
--- llvm/test/Support/encoding.ll
+++ llvm/test/Support/encoding.ll
@@ -1,9 +1,9 @@
 ; Checks if llc can deal with different char encodings.
 ; This is only required for z/OS.
 ;
-; UNSUPPORTED: !s390x-none-zos
+; REQUIRES: system-zos, systemz-registered-target
 ;
-; RUN: cat %s >%t && chtag -tc ISO8859-1 %t && llc %t -o - >/dev/null
+; RUN: cat %s >%t && chtag -tc ISO8859-1 %t && llc -mtriple=s390x-ibm-zos %t -o - >/dev/null
 ; RUN: iconv -f ISO8859-1 -t IBM-1047 <%s >%t && chtag -tc IBM-1047 %t && llc %t -o - >/dev/null
 ; RUN: iconv -f ISO8859-1 -t IBM-1047 <%s >%t && chtag -r %t && llc %t -o - >/dev/null
 
Index: llvm/test/MC/AsmParser/debug-no-source.s
===
--- llvm/test/MC/AsmParser/debug-no-source.s
+++ llvm/test/MC/AsmParser/debug-no-source.s
@@ -1,4 +1,4 @@
-// UNSUPPORTED: -zos
+// UNSUPPORTED: target={{.*}}-zos{{.*}}
 // REQUIRES: object-emission
 // RUN: llvm-mc %s | FileCheck %s
 
Index: clang/test/PCH/pending-ids.m
===
--- clang/test/PCH/pending-ids.m
+++ clang/test/PCH/pending-ids.m
@@ -1,4 +1,4 @@
-// UNSUPPORTED: -zos, target={{.*}}-aix{{.*}}
+// UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
 // Test for rdar://10278815
 
 // Without PCH
Index: clang/test/PCH/objcxx-ivar-class.mm
===

[clang] 7793e67 - [ZOS] Convert tests to check 'target={{.*}}-zos{{.*}}'

2022-12-12 Thread Paul Robinson via cfe-commits 

Author: Paul Robinson
Date: 2022-12-12T11:25:19-08:00
New Revision: 7793e676514bc102e97a993e90257e8628069a8b

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

LOG: [ZOS] Convert tests to check 'target={{.*}}-zos{{.*}}'

Also add 'system-zos' as a lit feature and use it where needed.

Part of the project to eliminate special handling for triples in lit
expressions.

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

Added: 


Modified: 
clang/test/Analysis/cfref_PR2519.c
clang/test/CodeGen/cfstring2.c
clang/test/Driver/as-version.s
clang/test/Import/forward-declared-objc-class/test.m
clang/test/Import/objc-arc/test-cleanup-object.m
clang/test/Import/objc-autoreleasepool/test.m
clang/test/Import/objc-definitions-in-expression/test.m
clang/test/Import/objc-method/test.m
clang/test/Import/objc-param-decl/test.m
clang/test/Import/objc-try-catch/test.m
clang/test/Modules/DebugInfoNamespace.cpp
clang/test/Modules/DebugInfoTransitiveImport.m
clang/test/Modules/ExtDebugInfo.cpp
clang/test/Modules/ExtDebugInfo.m
clang/test/Modules/ModuleDebugInfo.cpp
clang/test/Modules/ModuleDebugInfo.m
clang/test/Modules/ModuleDebugInfoDwoId.cpp
clang/test/Modules/ModuleModuleDebugInfo.cpp
clang/test/Modules/autolink.m
clang/test/Modules/autolinkTBD.m
clang/test/Modules/builtins.m
clang/test/Modules/clang_module_file_info.m
clang/test/Modules/cxx-irgen.cpp
clang/test/Modules/debug-info-moduleimport-in-module.m
clang/test/Modules/debug-info-moduleimport.m
clang/test/Modules/direct-module-import.m
clang/test/Modules/merge-anon-record-definition-in-objc.m
clang/test/Modules/merge-extension-ivars.m
clang/test/Modules/merge-objc-interface-visibility.m
clang/test/Modules/merge-objc-interface.m
clang/test/Modules/merge-record-definition-nonmodular.m
clang/test/Modules/merge-record-definition-visibility.m
clang/test/Modules/merge-record-definition.m
clang/test/Modules/module-debuginfo-prefix.m
clang/test/Modules/module-file-home-is-cwd.m
clang/test/Modules/module_file_info.m
clang/test/Modules/objc-initializer.m
clang/test/Modules/pch-used.m
clang/test/Modules/redecl-ivars.m
clang/test/Modules/use-exportas-for-link.m
clang/test/PCH/externally-retained.m
clang/test/PCH/irgen-rdar13114142.mm
clang/test/PCH/objc_container.m
clang/test/PCH/objc_literals.m
clang/test/PCH/objc_literals.mm
clang/test/PCH/objcxx-ivar-class.mm
clang/test/PCH/pending-ids.m
llvm/test/MC/AsmParser/debug-no-source.s
llvm/test/Support/encoding.ll
llvm/test/tools/llvm-mc/no_warnings.test
llvm/utils/lit/lit/llvm/config.py

Removed: 




diff  --git a/clang/test/Analysis/cfref_PR2519.c 
b/clang/test/Analysis/cfref_PR2519.c
index e63ac10fb5490..e90a2f17c75dc 100644
--- a/clang/test/Analysis/cfref_PR2519.c
+++ b/clang/test/Analysis/cfref_PR2519.c
@@ -1,4 +1,4 @@
-// UNSUPPORTED: -zos, target={{.*}}-aix{{.*}}
+// UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,osx.cocoa.RetainCount,alpha.core -verify %s
 // expected-no-diagnostics
 

diff  --git a/clang/test/CodeGen/cfstring2.c b/clang/test/CodeGen/cfstring2.c
index 74e3751fe49f8..da6190539e9e6 100644
--- a/clang/test/CodeGen/cfstring2.c
+++ b/clang/test/CodeGen/cfstring2.c
@@ -1,4 +1,4 @@
-// UNSUPPORTED: -zos, target={{.*}}-aix{{.*}}
+// UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
 // RUN: %clang_cc1 -emit-llvm %s -o %t
 
 typedef const struct __CFString * CFStringRef;

diff  --git a/clang/test/Driver/as-version.s b/clang/test/Driver/as-version.s
index 296f9e83ece93..a96b2b5762c65 100644
--- a/clang/test/Driver/as-version.s
+++ b/clang/test/Driver/as-version.s
@@ -1,6 +1,6 @@
 // Test version information.
 
-// UNSUPPORTED: -zos
+// UNSUPPORTED: target={{.*}}-zos{{.*}}
 // RUN: %clang -Wa,--version -c -fintegrated-as %s -o /dev/null \
 // RUN:   | FileCheck --check-prefix=IAS %s
 // IAS: clang version

diff  --git a/clang/test/Import/forward-declared-objc-class/test.m 
b/clang/test/Import/forward-declared-objc-class/test.m
index adf0c25d00cdd..c94c677dcef6a 100644
--- a/clang/test/Import/forward-declared-objc-class/test.m
+++ b/clang/test/Import/forward-declared-objc-class/test.m
@@ -1,4 +1,4 @@
-// UNSUPPORTED: -zos, target={{.*}}-aix{{.*}}
+// UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
 // RUN: clang-import-test -x objective-c++ -import %S/Inputs/S1.m --import 
%S/Inputs/S2.m --import %S/Inputs/S3.m -expression %s
 void expr() {
   MyClass *c = [MyClass fromInteger:3];

diff  --git a/clang/test/Import/objc-arc/test-cleanup-object.m 
b/clang/test/Import/objc-arc/test-cleanup-object.m
index a

[PATCH] D136594: [clangd] Add support for semantic token type "operator"

2022-12-12 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge added a comment.

Thanks, Christian, for your work on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136594

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

[PATCH] D138675: [flang] Add -ffast-math and -Ofast

2022-12-12 Thread Amy Kwan via Phabricator via cfe-commits 
amyk added a comment.

Hi,

The ppc64le-flang-rhel-clang  bot 
is also experiencing some failures with the `Driver/fast_math.f90` test case. 
In particular, `crtfastmath.o` is not found at all for `CHECK-CRT` as we can 
see in the failure details 
.

  llvm-project/flang/test/Driver/fast_math.f90:64:19: error: CHECK-CRT-SAME: 
expected string not found in input
  ! CHECK-CRT-SAME: crtfastmath.o

The output for the `CHECK-CRT` line on the bot looks like the following:

  
"/home/buildbots/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/build/bin/flang-new"
 "-fc1" "-triple" "powerpc64le-unknown-linux-gnu" "-emit-obj" 
"-mrelocation-model" "pic" "-pic-level" "2" "-pic-is-pie" "-ffast-math" 
"-target-cpu" "ppc64le" "-o" "/tmp/lit-tmp-x_okwzug/fast_math-e9ab49.o" "-x" 
"f95-cpp-input" 
"/home/buildbots/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/llvm-project/flang/test/Driver/fast_math.f90"
 
  
  "/usr/bin/ld" "-pie" "--hash-style=gnu" "--eh-frame-hdr" "-m" "elf64lppc" 
"-dynamic-linker" "/lib64/ld64.so.2" "-o" 
"/home/buildbots/ppc64le-flang-rhel-test/ppc64le-flang-rhel-clang-build/build/tools/flang/test/Driver/Output/fast_math.f90.tmp"
 "/usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64/Scrt1.o" 
"/usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64/crti.o" 
"/usr/lib/gcc/ppc64le-redhat-linux/8/crtbeginS.o" 
"-L/usr/lib/gcc/ppc64le-redhat-linux/8" 
"-L/usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64" "-L/lib/../lib64" 
"-L/usr/lib/../lib64" "-L/lib" "-L/usr/lib" 
"/tmp/lit-tmp-x_okwzug/fast_math-e9ab49.o" "-lFortran_main" "-lFortranRuntime" 
"-lFortranDecimal" "-lm" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" "-lc" 
"-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
"/usr/lib/gcc/ppc64le-redhat-linux/8/crtendS.o" 
"/usr/lib/gcc/ppc64le-redhat-linux/8/../../../../lib64/crtn.o"  
  

Would it be possible to follow up with a fix for this test case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138675

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

[PATCH] D138210: [clang] Require parameter pack to be last argument in concepts.

2022-12-12 Thread Luke Nihlen via Phabricator via cfe-commits 
luken-google closed this revision.
luken-google added a comment.

committed at afd800fc5679ccfe6f32b097586c658628500867 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138210

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

[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

2022-12-12 Thread Eli Friedman via Phabricator via cfe-commits 
efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:572
 PrioritizedCXXGlobalInits.push_back(std::make_pair(Key, Fn));
+  } else if (D->hasConstantInitialization() && !(D->hasAttr())) 
{
+OrderGlobalInitsOrStermFinalizers Key(201,

zahiraam wrote:
> efriedma wrote:
> > zahiraam wrote:
> > > efriedma wrote:
> > > > zahiraam wrote:
> > > > > efriedma wrote:
> > > > > > zahiraam wrote:
> > > > > > > efriedma wrote:
> > > > > > > > zahiraam wrote:
> > > > > > > > > efriedma wrote:
> > > > > > > > > > How is ConstInitAttr relevant here?
> > > > > > > > > This change made (without the !(D->hasAttr()) 
> > > > > > > > > made the LIT behavior of aix-static-init.cpp. The IR 
> > > > > > > > > generated for 
> > > > > > > > > namespace test3 {
> > > > > > > > >   struct Test3 {
> > > > > > > > > constexpr Test3() {};
> > > > > > > > > ~Test3() {};
> > > > > > > > >   };
> > > > > > > > > 
> > > > > > > > >   constinit Test3 t;
> > > > > > > > > } // namespace test3
> > > > > > > > > 
> > > > > > > > > was different. I would have thought that the change we made 
> > > > > > > > > for constexpr wouldn't affter constinit? 
> > > > > > > > I think the significant bit there isn't the use of constinit; 
> > > > > > > > it's the non-trivial destructor.  I think the priority 
> > > > > > > > modification should only affect constructors, not destructors.  
> > > > > > > > (Not sure how to make that work, at first glance.)
> > > > > > > Let's see if this is an acceptable solution.
> > > > > > To fake constant initialization, we need to initialize the variable 
> > > > > > before anything else runs.  But the rearranged prioritization isn't 
> > > > > > supposed to affect the destructor.  From [basic.start.term]: "If an 
> > > > > > object is initialized statically, the object is destroyed in the 
> > > > > > same order as if the object was dynamically initialized."
> > > > > > 
> > > > > > What you're doing here isn't exactly implementing that.  What 
> > > > > > you're doing here is delaying both the initialization and the 
> > > > > > destruction if the variable has a non-trivial destructor.  We need 
> > > > > > to separate the two to get the behavior we want.
> > > > > Could we consider adding a field to EvaluatedStmt called 
> > > > > "HasTrivialDestrutor" and only perform the prioritization change when 
> > > > > !D->HasTrivialDesctructor?  Instead of using the test for 
> > > > > D->hasConstantInitialization(). This seems to be englobing to many 
> > > > > cases.
> > > > > 
> > > > > I considered returning null for HasConstantInitialization in case of 
> > > > > var has a non-trivial destructor but that doesn't seem to work.
> > > > Once you separate out the initialization from the destruction, the rest 
> > > > should come together naturally, I think? I'm not sure what other cases 
> > > > could be caught by hasConstantInitialization().
> > > Does this change accomplish this? Thanks.
> > When a global variable is dynamically initialized, there are two parts to 
> > the initialization:
> > 
> > - Constructing the variable (calling the constructor, or equivalent)
> > - Registering the destructor with the runtime (atexit on Windows)
> > 
> > If an object is initialized statically, the two parts are separated: the 
> > variable is emitted already initialized by the compiler.  But we still 
> > register the destructor at the same time, in the same way.
> > 
> > If a dllimport object is initialized statically, we need to make it appear 
> > to user code that the same thing happened.  So we need to initialize the 
> > object early, but we need to register the destructor at the same time we 
> > would have otherwise registered it.  To make this work, we need two 
> > separate initializer functions with different priorities.
> Thanks for the clarification. I am slowly getting it, I think.
> 
> In terms of IR, for example for this:
>   struct Test3 {
> constexpr Test3() {};
> ~Test3() {};
>   };
> 
>   constinit Test3 t;
> 
> Are we looking for something like this (partial IR):
> 
>   @t = global %struct.Test3 undef
>   @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, 
> ptr } { i32 **201**, ptr @_GLOBAL__I_000201, ptr null }]
>   @llvm.global_dtors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, 
> ptr } { i32 **65535**, ptr @_GLOBAL__D_a, ptr null }]
>   define internal void @__cxx_global_var_init() {
>   entry:
> call void @_ZN5Test3C1Ev(ptr noundef nonnull align 1 dereferenceable(1) 
> @t)
> %0 = call i32 @atexit(ptr @__dtor_t) 
> ret void
>   }
>   define internal void @__finalize_t() {
>%0 = call i32 @unatexit(ptr @__dtor_t) 
>%needs_destruct = icmp eq i32 %0, 0
>br i1 %needs_destruct, label %destruct.call, label %destruct.end
> 
>   destruct.call:  
> call void @__dtor_t()
>   .}
>   }
> 
> Or
> 
>   define internal void @__cxx_global_var_init()  {
>   entry:
> call void

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

2022-12-12 Thread Ulrich Weigand via Phabricator via cfe-commits 
uweigand accepted this revision.
uweigand added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


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

https://reviews.llvm.org/D139444

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

[PATCH] D139801: [clang-format] Adds 'friend' to QualifierOrder.

2022-12-12 Thread Micah Weston via Phabricator via cfe-commits 
red1bluelost added a comment.

In D139801#3988790 , 
@HazardyKnusperkeks wrote:

> If only the first, I'd say we do that without adding this option.

I'll work on doing this without adding the option. (Just reread and noticed you 
had answered what I asked in my previous comment.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139801

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

[PATCH] D139485: [LLVM] Remove redundant .c_str() and .get() calls where they are not needed.

2022-12-12 Thread Shafik Yaghmour via Phabricator via cfe-commits 
shafik added inline comments.



Comment at: lldb/source/API/SBPlatform.cpp:14
 #include "lldb/API/SBLaunchInfo.h"
-#include "lldb/API/SBPlatform.h"
 #include "lldb/API/SBUnixSignals.h"

These redundant  header removals looks unrelated and should be done as a 
separate NFC change.


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

https://reviews.llvm.org/D139485

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

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

2022-12-12 Thread Tom Honermann via Phabricator via cfe-commits 
tahonermann accepted this revision.
tahonermann added a comment.
This revision is now accepted and ready to land.

Builds passed now. I think this looks good.


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

https://reviews.llvm.org/D139148

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

[PATCH] D136554: Implement CWG2631

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

I'm fine with relanding and seeing if anything else pops up


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] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

2022-12-12 Thread Paul Robinson via Phabricator via cfe-commits 
probinson added a comment.

Thanks @Kai and @uweigand, good to know a z/OS bot is in the works. Hope this 
patch now meets your needs.


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

https://reviews.llvm.org/D139444

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

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

2022-12-12 Thread Paul Robinson via Phabricator via cfe-commits 
probinson updated this revision to Diff 482180.
probinson added a comment.
Herald added a subscriber: delcypher.

Define 'system-zos' and use it in the one test that needs it.


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

https://reviews.llvm.org/D139444

Files:
  clang/test/Analysis/cfref_PR2519.c
  clang/test/CodeGen/cfstring2.c
  clang/test/Driver/as-version.s
  clang/test/Import/forward-declared-objc-class/test.m
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Import/objc-autoreleasepool/test.m
  clang/test/Import/objc-definitions-in-expression/test.m
  clang/test/Import/objc-method/test.m
  clang/test/Import/objc-param-decl/test.m
  clang/test/Import/objc-try-catch/test.m
  clang/test/Modules/DebugInfoNamespace.cpp
  clang/test/Modules/DebugInfoTransitiveImport.m
  clang/test/Modules/ExtDebugInfo.cpp
  clang/test/Modules/ExtDebugInfo.m
  clang/test/Modules/ModuleDebugInfo.cpp
  clang/test/Modules/ModuleDebugInfo.m
  clang/test/Modules/ModuleDebugInfoDwoId.cpp
  clang/test/Modules/ModuleModuleDebugInfo.cpp
  clang/test/Modules/autolink.m
  clang/test/Modules/autolinkTBD.m
  clang/test/Modules/builtins.m
  clang/test/Modules/clang_module_file_info.m
  clang/test/Modules/cxx-irgen.cpp
  clang/test/Modules/debug-info-moduleimport-in-module.m
  clang/test/Modules/debug-info-moduleimport.m
  clang/test/Modules/direct-module-import.m
  clang/test/Modules/merge-anon-record-definition-in-objc.m
  clang/test/Modules/merge-extension-ivars.m
  clang/test/Modules/merge-objc-interface-visibility.m
  clang/test/Modules/merge-objc-interface.m
  clang/test/Modules/merge-record-definition-nonmodular.m
  clang/test/Modules/merge-record-definition-visibility.m
  clang/test/Modules/merge-record-definition.m
  clang/test/Modules/module-debuginfo-prefix.m
  clang/test/Modules/module-file-home-is-cwd.m
  clang/test/Modules/module_file_info.m
  clang/test/Modules/objc-initializer.m
  clang/test/Modules/pch-used.m
  clang/test/Modules/redecl-ivars.m
  clang/test/Modules/use-exportas-for-link.m
  clang/test/PCH/externally-retained.m
  clang/test/PCH/irgen-rdar13114142.mm
  clang/test/PCH/objc_container.m
  clang/test/PCH/objc_literals.m
  clang/test/PCH/objc_literals.mm
  clang/test/PCH/objcxx-ivar-class.mm
  clang/test/PCH/pending-ids.m
  llvm/test/MC/AsmParser/debug-no-source.s
  llvm/test/Support/encoding.ll
  llvm/test/tools/llvm-mc/no_warnings.test
  llvm/utils/lit/lit/llvm/config.py

Index: llvm/utils/lit/lit/llvm/config.py
===
--- llvm/utils/lit/lit/llvm/config.py
+++ llvm/utils/lit/lit/llvm/config.py
@@ -88,6 +88,8 @@
 features.add('system-aix')
 elif platform.system() == 'SunOS':
 features.add('system-solaris')
+elif platform.system() == 'OS/390':
+features.add('system-zos')
 
 # Native compilation: host arch == default triple arch
 # Both of these values should probably be in every site config (e.g. as
Index: llvm/test/tools/llvm-mc/no_warnings.test
===
--- llvm/test/tools/llvm-mc/no_warnings.test
+++ llvm/test/tools/llvm-mc/no_warnings.test
@@ -1,4 +1,4 @@
-# UNSUPPORTED: -zos
+# UNSUPPORTED: target={{.*}}-zos{{.*}}
 # RUN: llvm-mc --no-warn %s 2>&1 | FileCheck %s
 
 # CHECK-NOT: warning:
Index: llvm/test/Support/encoding.ll
===
--- llvm/test/Support/encoding.ll
+++ llvm/test/Support/encoding.ll
@@ -1,9 +1,9 @@
 ; Checks if llc can deal with different char encodings.
 ; This is only required for z/OS.
 ;
-; UNSUPPORTED: !s390x-none-zos
+; REQUIRES: system-zos, systemz-registered-target
 ;
-; RUN: cat %s >%t && chtag -tc ISO8859-1 %t && llc %t -o - >/dev/null
+; RUN: cat %s >%t && chtag -tc ISO8859-1 %t && llc -mtriple=s390x-ibm-zos %t -o - >/dev/null
 ; RUN: iconv -f ISO8859-1 -t IBM-1047 <%s >%t && chtag -tc IBM-1047 %t && llc %t -o - >/dev/null
 ; RUN: iconv -f ISO8859-1 -t IBM-1047 <%s >%t && chtag -r %t && llc %t -o - >/dev/null
 
Index: llvm/test/MC/AsmParser/debug-no-source.s
===
--- llvm/test/MC/AsmParser/debug-no-source.s
+++ llvm/test/MC/AsmParser/debug-no-source.s
@@ -1,4 +1,4 @@
-// UNSUPPORTED: -zos
+// UNSUPPORTED: target={{.*}}-zos{{.*}}
 // REQUIRES: object-emission
 // RUN: llvm-mc %s | FileCheck %s
 
Index: clang/test/PCH/pending-ids.m
===
--- clang/test/PCH/pending-ids.m
+++ clang/test/PCH/pending-ids.m
@@ -1,4 +1,4 @@
-// UNSUPPORTED: -zos, target={{.*}}-aix{{.*}}
+// UNSUPPORTED: target={{.*}}-zos{{.*}}, target={{.*}}-aix{{.*}}
 // Test for rdar://10278815
 
 // Without PCH
Index: clang/test/PCH/objcxx-ivar-class.mm
===
--- clang/test/PCH/objcxx-ivar-class.mm
+++ clang/test/PCH/objcxx-ivar-class.mm
@@ -1,4 +1,4 @@
-

[clang] fc7b8e7 - [test] Fix dr324.c again for non-writeable source directories

2022-12-12 Thread Jordan Rupprecht via cfe-commits 

Author: Jordan Rupprecht
Date: 2022-12-12T10:11:19-08:00
New Revision: fc7b8e71312f89986be1a7cc67377a2decfcf337

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

LOG: [test] Fix dr324.c again for non-writeable source directories

In general, the source tree is not assumed to be writeable, so modifying `%s` 
does not work for all CI systems. Instead of touching `%s`, copy it to a 
writeable dir using `%t`, and touch it there.
Actually, `dr0xx.c` isn't really needed at all, so just create a new `dep.c` 
file in the build tree.

This was recently added in cb088e8c3abf30456e2891f90b5194d0070c387a, fixed in 
1481fcf780bde7b115aa395064d71749b1a40889, and fixed again in 
d16c59013056f1bf8844ded8faeb0cf01b1c3613.

Added: 


Modified: 
clang/test/C/drs/dr324.c

Removed: 




diff  --git a/clang/test/C/drs/dr324.c b/clang/test/C/drs/dr324.c
index 2d75853e5af7c..585041b0365f3 100644
--- a/clang/test/C/drs/dr324.c
+++ b/clang/test/C/drs/dr324.c
@@ -1,9 +1,18 @@
-/* RUN: touch %s
-   RUN: %clang_cc1 -std=c89 -fsyntax-only -fms-extensions -pedantic -verify %s
-   RUN: %clang_cc1 -std=c99 -fsyntax-only -fms-extensions -pedantic -verify %s
-   RUN: %clang_cc1 -std=c11 -fsyntax-only -fms-extensions -pedantic -verify %s
-   RUN: %clang_cc1 -std=c17 -fsyntax-only -fms-extensions -pedantic -verify %s
-   RUN: %clang_cc1 -std=c2x -fsyntax-only -fms-extensions -pedantic -verify %s
+/* RUN: rm -rf %t && mkdir %t
+   RUN: cp %s %t/dr324.c
+
+   Note: this file (dr324.c) must be newer than the file used for the
+ dependency pragma (dep.c), otherwise we get an unrelated "current file is
+ older than dependency" warning. Touch dep.c first to make sure it's
+ always older.
+   RUN: touch %t/dep.c
+   RUN: touch %t/dr324.c
+
+   RUN: %clang_cc1 -std=c89 -fsyntax-only -fms-extensions -pedantic -verify 
%t/dr324.c
+   RUN: %clang_cc1 -std=c99 -fsyntax-only -fms-extensions -pedantic -verify 
%t/dr324.c
+   RUN: %clang_cc1 -std=c11 -fsyntax-only -fms-extensions -pedantic -verify 
%t/dr324.c
+   RUN: %clang_cc1 -std=c17 -fsyntax-only -fms-extensions -pedantic -verify 
%t/dr324.c
+   RUN: %clang_cc1 -std=c2x -fsyntax-only -fms-extensions -pedantic -verify 
%t/dr324.c
  */
 
 /* WG14 DR324: yes
@@ -25,7 +34,7 @@ char lit_char = '\y';   /* expected-warning {{unknown 
escape sequence '\y'}}
 /* This test only makes sense on Windows targets, where the backslash is a 
valid
  * path separator.
  */
-#pragma GCC dependency "oops\..\dr0xx.c"
+#pragma GCC dependency "oops\..\dep.c"
 #endif
 #pragma message("this has a \t tab escape and an invalid \d escape") /* 
expected-warning {{this has a   tab escape and an invalid d escape}}
 
expected-warning {{unknown escape sequence '\d'}}



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

[PATCH] D139801: [clang-format] Adds 'friend' to QualifierOrder.

2022-12-12 Thread Micah Weston via Phabricator via cfe-commits 
red1bluelost added a comment.

I would always want it first. I agree that `friend` would always go first. Had 
I not accidentally typed in the wrong order I probably wouldn't have know 
`friend` could be in any position.

In the case of putting `friend` always first, would we want a user facing 
option to enable/disable moving it to first, do it always without an user 
option, or something else?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139801

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

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

2022-12-12 Thread Kai Nacke via Phabricator via cfe-commits 
Kai added a comment.

In D139444#3988544 , @probinson wrote:

> Searching the buildbot console page for 'zos' turns up nothing; 's390' turns 
> up clang-s390x-linux, clang-s390x-linux-lnt, mlir-s390x-linux.  Are there any 
> zos hosted bots?  I.e., is this test ever actually run?  Maybe there are only 
> downstream zos bots, in which case this should be a downstream test instead?

We are in the process of setting up a buildbot for z/OS. I would appreciate to 
have this upstream.


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

https://reviews.llvm.org/D139444

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

[clang] b7ada67 - [lit] Use 'target=' in a few more places

2022-12-12 Thread Paul Robinson via cfe-commits 

Author: Paul Robinson
Date: 2022-12-12T09:58:38-08:00
New Revision: b7ada67f1c28ae67c61578dd3bc8acb4d8d55d28

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

LOG: [lit] Use 'target=' in a few more places

Missed these on the first pass.

Part of the project to eliminate special handling for triples in lit
expressions.

Added: 


Modified: 
clang/test/CodeGen/volatile-1.c
clang/test/Modules/hidden-duplicates.m
clang/test/Modules/override.m

Removed: 




diff  --git a/clang/test/CodeGen/volatile-1.c b/clang/test/CodeGen/volatile-1.c
index d27abbfdbef1..cd919c78989f 100644
--- a/clang/test/CodeGen/volatile-1.c
+++ b/clang/test/CodeGen/volatile-1.c
@@ -1,4 +1,4 @@
-// XFAIL: aarch64-pc-windows-msvc
+// XFAIL: target=aarch64-pc-windows-msvc
 // RUN: %clang_cc1 -Wno-return-type -Wno-unused-value -emit-llvm %s -w -o - | 
FileCheck %s
 
 // CHECK: @i = {{(dso_local )?}}global [[INT:i[0-9]+]] 0

diff  --git a/clang/test/Modules/hidden-duplicates.m 
b/clang/test/Modules/hidden-duplicates.m
index ec9549dab53e..423654502fd8 100644
--- a/clang/test/Modules/hidden-duplicates.m
+++ b/clang/test/Modules/hidden-duplicates.m
@@ -1,4 +1,4 @@
-// XFAIL: aix, -zos
+// XFAIL: target={{.*}}-aix{{.*}}, target={{.*}}-zos{{.*}}
 // RUN: rm -rf %t
 // RUN: split-file %s %t
 // RUN: %clang_cc1 -emit-llvm -o %t/test.bc -I %t/include %t/test.m -verify \

diff  --git a/clang/test/Modules/override.m b/clang/test/Modules/override.m
index 247c2be76ee1..5725ef16552d 100644
--- a/clang/test/Modules/override.m
+++ b/clang/test/Modules/override.m
@@ -1,4 +1,4 @@
-// UNSUPPORTED: -aix
+// UNSUPPORTED: target={{.*}}-aix{{.*}}
 // RUN: rm -rf %t
 // RUN: split-file %s %t
 // RUN: %clang_cc1 -fsyntax-only -I%t/include %t/test.m \



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

[PATCH] D139444: [ZOS] Convert tests to check 'target={{.*}}-zos'

2022-12-12 Thread Kai Nacke via Phabricator via cfe-commits 
Kai added a comment.

In D139444#3978189 , @uweigand wrote:

> In D139444#3975182 , @probinson 
> wrote:
>
>> The changes in this patch assume that there aren't any possible suffixes 
>> after the `-zos` part of the triple (no version numbers, like you might find 
>> with darwin or macos, and nothing like `-elf` or `-eabi` like some targets 
>> have).  If there are suffixes, I'll happily revise to put `{{.*}}` after 
>> everything.
>
> I think for consistency with other targets, and to be safe for future 
> extensions of the target triple, it would be better to add the `{{.*}}`

There will be suffixes after the `-zos` part. Thanks for adding the pattern at 
the end.


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

https://reviews.llvm.org/D139444

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

  1   2   3   >