[PATCH] D158883: [Matrix] Try to emit fmuladd for both vector and matrix types

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

In D158883#4635997 , @uweigand wrote:

> The newly added test cases in ffp-model.c fail on SystemZ, making CI red:

Should be fixed, thanks for the report and sorry for the delay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158883

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

[PATCH] D158883: [Matrix] Try to emit fmuladd for both vector and matrix types

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc987f9d7fdc7: [Matrix] Try to emit fmuladd for both vector 
and matrix types (authored by thegameg).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158883

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/ffp-model.c

Index: clang/test/CodeGen/ffp-model.c
===
--- clang/test/CodeGen/ffp-model.c
+++ clang/test/CodeGen/ffp-model.c
@@ -1,18 +1,18 @@
 // REQUIRES: x86-registered-target
-// RUN: %clang -S -emit-llvm -ffp-model=fast -emit-llvm %s -o - \
+// RUN: %clang -S -emit-llvm -fenable-matrix -ffp-model=fast %s -o - \
 // RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-FAST
 
-// RUN: %clang -S -emit-llvm -ffp-model=precise %s -o - \
+// RUN: %clang -S -emit-llvm -fenable-matrix -ffp-model=precise %s -o - \
 // RUN: | FileCheck %s --check-prefixes=CHECK,CHECK-PRECISE
 
-// RUN: %clang -S -emit-llvm -ffp-model=strict %s -o - \
+// RUN: %clang -S -emit-llvm -fenable-matrix -ffp-model=strict %s -o - \
 // RUN: -target x86_64 | FileCheck %s --check-prefixes=CHECK,CHECK-STRICT
 
-// RUN: %clang -S -emit-llvm -ffp-model=strict -ffast-math \
+// RUN: %clang -S -emit-llvm -fenable-matrix -ffp-model=strict -ffast-math \
 // RUN: -target x86_64 %s -o - | FileCheck %s \
 // RUN: --check-prefixes CHECK,CHECK-STRICT-FAST
 
-// RUN: %clang -S -emit-llvm -ffp-model=precise -ffast-math \
+// RUN: %clang -S -emit-llvm -fenable-matrix -ffp-model=precise -ffast-math \
 // RUN: %s -o - | FileCheck %s --check-prefixes CHECK,CHECK-FAST1
 
 float mymuladd(float x, float y, float z) {
@@ -46,3 +46,105 @@
   // CHECK-FAST1: load float, ptr {{.*}}
   // CHECK-FAST1: fadd fast float {{.*}}, {{.*}}
 }
+
+typedef float __attribute__((ext_vector_type(2))) v2f;
+
+v2f my_vec_muladd(v2f x, float y, v2f z) {
+  // CHECK: define{{.*}} @my_vec_muladd
+  return x * y + z;
+
+  // CHECK-FAST: fmul fast <2 x float>
+  // CHECK-FAST: load <2 x float>, ptr
+  // CHECK-FAST: fadd fast <2 x float>
+
+  // CHECK-PRECISE: load <2 x float>, ptr
+  // CHECK-PRECISE: load float, ptr
+  // CHECK-PRECISE: load <2 x float>, ptr
+  // CHECK-PRECISE: call <2 x float> @llvm.fmuladd.v2f32(<2 x float> {{.*}}, <2 x float> {{.*}}, <2 x float> {{.*}})
+
+  // CHECK-STRICT: load <2 x float>, ptr
+  // CHECK-STRICT: load float, ptr
+  // CHECK-STRICT: call <2 x float> @llvm.experimental.constrained.fmul.v2f32(<2 x float> {{.*}}, <2 x float> {{.*}}, {{.*}})
+  // CHECK-STRICT: load <2 x float>, ptr
+  // CHECK-STRICT: call <2 x float> @llvm.experimental.constrained.fadd.v2f32(<2 x float> {{.*}}, <2 x float> {{.*}}, {{.*}})
+
+  // CHECK-STRICT-FAST: load <2 x float>, ptr
+  // CHECK-STRICT-FAST: load float, ptr
+  // CHECK-STRICT-FAST: fmul fast <2 x float> {{.*}}, {{.*}}
+  // CHECK-STRICT-FAST: load <2 x float>, ptr
+  // CHECK-STRICT-FAST: fadd fast <2 x float> {{.*}}, {{.*}}
+
+  // CHECK-FAST1: load <2 x float>, ptr
+  // CHECK-FAST1: load float, ptr
+  // CHECK-FAST1: fmul fast <2 x float> {{.*}}, {{.*}}
+  // CHECK-FAST1: load <2 x float>, ptr {{.*}}
+  // CHECK-FAST1: fadd fast <2 x float> {{.*}}, {{.*}}
+}
+
+typedef float __attribute__((matrix_type(2, 1))) m21f;
+
+m21f my_m21_muladd(m21f x, float y, m21f z) {
+  // CHECK: define{{.*}} <2 x float> @my_m21_muladd
+  return x * y + z;
+
+  // CHECK-FAST: fmul fast <2 x float>
+  // CHECK-FAST: load <2 x float>, ptr
+  // CHECK-FAST: fadd fast <2 x float>
+
+  // CHECK-PRECISE: load <2 x float>, ptr
+  // CHECK-PRECISE: load float, ptr
+  // CHECK-PRECISE: load <2 x float>, ptr
+  // CHECK-PRECISE: call <2 x float> @llvm.fmuladd.v2f32(<2 x float> {{.*}}, <2 x float> {{.*}}, <2 x float> {{.*}})
+
+  // CHECK-STRICT: load <2 x float>, ptr
+  // CHECK-STRICT: load float, ptr
+  // CHECK-STRICT: call <2 x float> @llvm.experimental.constrained.fmul.v2f32(<2 x float> {{.*}}, <2 x float> {{.*}}, {{.*}})
+  // CHECK-STRICT: load <2 x float>, ptr
+  // CHECK-STRICT: call <2 x float> @llvm.experimental.constrained.fadd.v2f32(<2 x float> {{.*}}, <2 x float> {{.*}}, {{.*}})
+
+  // CHECK-STRICT-FAST: load <2 x float>, ptr
+  // CHECK-STRICT-FAST: load float, ptr
+  // CHECK-STRICT-FAST: fmul fast <2 x float> {{.*}}, {{.*}}
+  // CHECK-STRICT-FAST: load <2 x float>, ptr
+  // CHECK-STRICT-FAST: fadd fast <2 x float> {{.*}}, {{.*}}
+
+  // CHECK-FAST1: load <2 x float>, ptr
+  // CHECK-FAST1: load float, ptr
+  // CHECK-FAST1: fmul fast <2 x float> {{.*}}, {{.*}}
+  // CHECK-FAST1: load <2 x float>, ptr {{.*}}
+  // CHECK-FAST1: fadd fast <2 x float> {{.*}}, {{.*}}
+}
+
+typedef float __attribute__((matrix_type(2, 2))) m22f;
+
+m22f my_m22_muladd(m22f x, float y, m22f z) {
+  // CHECK: define{{.*}} <4 x float> @my_m22_muladd
+  return x * y + z;
+
+  // CHECK-FAST: fmul fast <4 x float>
+  // 

[PATCH] D156515: [RemarkUtil] Refactor llvm-remarkutil to include size-diff

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

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

Clean, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156515

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

[PATCH] D153746: [Remarks] Make sure -fdiagnostics-hotness-threshold implies -fdiagnostics-show-hotness

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

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

Makes sense, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153746

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

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

In D135488#4049050 , @nickdesaulniers 
wrote:

> In D135488#4049035 , @paulkirth 
> wrote:
>
>> Actually if we add
>>
>>   if (!isFunctionInPrintList(MF.getName()))
>>return false;
>>
>> we can filter by name

I would rather have a more generic mechanism for remarks or diagnostics in 
general. Even if it uses `isFunctionInPrintList`, I'd rather have a real flag 
that doesn't require `-mllvm`.

> Does name mangling complicate that? Perhaps a C++ user would give an 
> unmangled name, but MF would be looking at mangled names?

Agreed. A regex would help a little there.

> Anyways, it's not a pressing issue. I won't block this patch on that. I just 
> redirect all the output to a file then scan that.

You can easily process the yaml output from `-fsave-optimization-record` with 
the `optrecord` module:

  import optrecord
  import re
  
  all_remarks, file_remarks, _ = 
optrecord.gather_results(optrecord.find_opt_files(), 1, False)
  for r in optrecord.itervalues(all_remarks):
if re.match(, r.Function):
  print(r)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488

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

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

In D135488#4048854 , @paulkirth wrote:

> In D135488#4048380 , 
> @nickdesaulniers wrote:
>
>> It would be really nice if we could limit this to a specific function 
>> somehow.
>
> I think you can do that, right ?
> see: 
> https://llvm.org/docs/Remarks.html#cmdoption-pass-remarks-filter

This filters on the `Name` of the remark, so here it would be 
`stack-frame-layout`.

I think a `-Rpass-func-filter=` would be a great addition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488

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

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

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

Looks great with the leftover minor changes, feel free to land this, thanks! 
I'll give this a try internally and provide feedback if any.




Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:55
+Variable,   // a Slot used to store a local data (could be a tmp)
+Error   // Its an error for a slot to have this type
+  };

paulkirth wrote:
> thegameg wrote:
> > 
> Good catch! TY
I also suggested `Invalid` instead of `Error` but it's up to you.



Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:110
+
+  std::string genStackOffsetPrefix(int I) const {
+// Negative offsets will print a leading `-`, so only add `+`

paulkirth wrote:
> thegameg wrote:
> > Is this still worth being a separate function?
> yeah, probably not 
Looks like this stayed around unused


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488

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

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

In D135488#4044437 , @paulkirth wrote:

> BTW, is there a way to nest some of the items? Ideally we'd be able to have a 
> `Slot` in the YAML that contains all the various data, similar to how 
> `DebugLoc` is a more complex object with fields for `File`, `Line`, and 
> `Column`. That way we could group all the data for each slot including 
> variable locations.
>
> I know how that would look in YAML, but I'm unaware of how we'd do that with 
> the existing remarks interfaces... or if doing so would massively change the 
> CLI output. any pointers here?

Unfortunately no, there is no easy way to do that right now, but I agree it 
would be nice. The `Args` seem easy enough to process that I wouldn't bother 
trying to group them. `optrecord.py` (and libRemarks) will handle it in the 
right order so the users can easily work with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488

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

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

This looks great, thanks for updating this! A few more comments inline.




Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:55
+Variable,   // a Slot used to store a local data (could be a tmp)
+Error   // Its an error for a slot to have this type
+  };





Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:65
+
+SlotData(const MachineFrameInfo , const int ValOffset, const int Idx) {
+  Slot = Idx;





Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:110
+
+  std::string genStackOffsetPrefix(int I) const {
+// Negative offsets will print a leading `-`, so only add `+`

Is this still worth being a separate function?



Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:128
+
+  void emitLayoutRemark(const MachineFunction , StringRef RemarkName,
+StringRef RemarkLabel, StringRef Data,

Unused?



Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:179
+const TargetFrameLowering *FI = MF.getSubtarget().getFrameLowering();
+const int ValOffset = (FI ? FI->getOffsetOfLocalArea() : 0);
+const unsigned int NumObj = MFI.getNumObjects();

Can you add a comment on what `ValOffset` is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488

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

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

I don't think I understand why we can't achieve B with remarks? In C and D you 
generate one remark for each line, can't we generate a single multi-line remark 
instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488

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

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

In D135488#4024713 , @paulkirth wrote:

> @arsenm @thegameg @nickdesaulniers @dblaikie @phosek Can we reach a consensus 
> here on how to output this kind of information? I feel like I've been told to 
> move towards remarks as the output method, but that the current diagnostic 
> that tries to lay out the stack visually isn't a good fit since remarks are 
> also serialized ... I'm not all that convinced that providing output other 
> than a visual layout for this information is all that useful in this 
> particular case, but I don't have an issue with supporting it either.  I 
> think this is especially true, since memory layouts are tricky to reason 
> about.
>
> For that reason, I'm pretty sure we want to actually //show// the user the 
> layout directly in the diagnostic. My concern is that if we change the output 
> to better fit within the remarks infrastructure, we lose an effective way to 
> show users what's happening. If we take away the visual representation, then 
> we'll end up needing to run a separate tool and post-process the serialized 
> output to have a user make any real sense of how things are layed out. That 
> seems like a pretty bad user experience, so I'd much rather find a way to 
> have the compiler emit this information directly.
>
> Does anyone have thoughts here on how to move forward?

I think remarks are the right way to go with this. They provide a pretty 
flexible way to emit both strings (for formatting and visual representations) 
and machine-readable data through `ore::NV` entries. We just need to find a 
consensus on how it looks like in both the command-line and the serialized mode.




Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:178
+
+emitHeaderRemark(MF);
+

paulkirth wrote:
> thegameg wrote:
> > From what I can see, you've focused on the `-Rpass` output using 
> > diagnostics and tried to emit a pretty-printed version for that on the 
> > command line.
> > 
> > We use remarks through their serialized version as well, through 
> > `-fsave-optimization-record` which will emit a YAML file that can be used 
> > in scripts and other post-processing tools.
> > 
> > I think this should be something in between where it looks user-friendly on 
> > the command-line but also easy to post-process.
> > 
> > One way would be to do something similar to [[ 
> > https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp
> >  | the memory op remarks ]], which are used here: [[ 
> > https://github.com/llvm/llvm-project/blob/main/llvm/test/Transforms/Util/trivial-auto-var-init-call.ll
> >  | llvm/test/Transforms/Util/trivial-auto-var-init-call.ll ]].
> > 
> > I could see something where you emit a remark for each slot (+ location), 
> > with `ore::NV` used for each piece of information that is useful, something 
> > like:
> > 
> > ```
> > ORE << MachineOptimizationRemarkAnalysis(...) << "Stack slot: offset: " << 
> > ore::NV("Offset", D.offset)
> > 
> > << "type: " << ore::NV("Type", type)
> > [...]
> > ```
> > 
> > and could generate something like:
> > 
> > ```
> >  --- !Analysis
> >  Pass:stack-frame-layout
> >  Name:StackSlot
> >  Function:stackSizeWarning
> >  Args:
> >- String:  'Stack slot: offset: '
> >- Offset: '[SP-8]'
> >- String:  ', type: '
> >- Type:'spill'
> >- String:  ', align: '
> >- Align:   '16'
> >- String:  ', size: '
> >- Align:   '8'
> >  ...
> > ```
> > 
> > which would look like this on the command line:
> > 
> > ```
> > remark: Stack slot: offset: [SP-8], type: spill, align: 16, size 8
> > ```
> > 
> Thanks for the suggestion. While I understand the desire to make the output 
> more machine readable, I don't think this is a good place to do so. Layouts 
> are hard to reason about and there's actually a fairly decent way we can 
> display this to users and convey exactly where things are. The entire point 
> of this patch was to give a somewhat visual representation to how the stack 
> is layed out, and help debug stack layout issues. It's one of the reasons I 
> didn't originally do this with remarks, but there's been a fair amount of 
> discussion to this point already w/in this patch. 
> 
> If this isn't a good fit for remarks with the current format, then I'm kind 
> of stuck on how to satisfy the various requirements on how to output and 
> display this kind of information...
> 
> 
> 
I don't see a huge difference between:

```
remark: OffsetAlign Size
remark: [SP-8]  Spill 168
remark: [SP-16] Spill 8 8
remark: [SP-24] Spill 168
```

and

```
remark: Stack slot: offset: [SP-8], type: spill, align: 16, size 8
remark: Stack slot: offset: 

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added inline comments.



Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:92
+   ())
+ << "FunctionName: " << ore::NV("FunctionName", MF.getName());
+});

Why are we emitting the function name? In the serialized remarks 
(`-fsave-optimization-record`) it comes in the `Function` field, and in the 
diagnostics (`-Rpass*`) it uses debug info to show the source around it.

if it's for testing only, you can test using the serialized remarks with YAML.



Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:124
+.str();
+emitLayoutRemark(MF, "Stack Layout", "StackLayout", Slot);
+  }

We usually use identifiers for remark names, so here `StackLayout` instead of 
`Stack Layout`.



Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:178
+
+emitHeaderRemark(MF);
+

From what I can see, you've focused on the `-Rpass` output using diagnostics 
and tried to emit a pretty-printed version for that on the command line.

We use remarks through their serialized version as well, through 
`-fsave-optimization-record` which will emit a YAML file that can be used in 
scripts and other post-processing tools.

I think this should be something in between where it looks user-friendly on the 
command-line but also easy to post-process.

One way would be to do something similar to [[ 
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/MemoryOpRemark.cpp
 | the memory op remarks ]], which are used here: [[ 
https://github.com/llvm/llvm-project/blob/main/llvm/test/Transforms/Util/trivial-auto-var-init-call.ll
 | llvm/test/Transforms/Util/trivial-auto-var-init-call.ll ]].

I could see something where you emit a remark for each slot (+ location), with 
`ore::NV` used for each piece of information that is useful, something like:

```
ORE << MachineOptimizationRemarkAnalysis(...) << "Stack slot: offset: " << 
ore::NV("Offset", D.offset)

<< "type: " << ore::NV("Type", type)
[...]
```

and could generate something like:

```
 --- !Analysis
 Pass:stack-frame-layout
 Name:StackSlot
 Function:stackSizeWarning
 Args:
   - String:  'Stack slot: offset: '
   - Offset: '[SP-8]'
   - String:  ', type: '
   - Type:'spill'
   - String:  ', align: '
   - Align:   '16'
   - String:  ', size: '
   - Align:   '8'
 ...
```

which would look like this on the command line:

```
remark: Stack slot: offset: [SP-8], type: spill, align: 16, size 8
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488

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

[PATCH] D135488: [codegen] Add a remarks based Stack Layout Analysis pass

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

This is great! Any chance we can use `MachineFrameInfo::StackProtectorIdx` to 
annotate the slot that is reserved for the stack protector?




Comment at: llvm/lib/CodeGen/StackFrameLayoutAnalysisPass.cpp:84
+ORE = ().getORE();
+if (!ORE)
+  return false;

I don't think this should ever be null.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135488

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

[PATCH] D100901: [CMake][llvm] avoid conflict w/ (and use when available) new builtin check_linker_flag

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG06d06f2f6403: [CMake][llvm] avoid conflict w/ (and use when 
available) new builtin… (authored by radford, committed by thegameg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100901

Files:
  clang/tools/driver/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/CheckLinkerFlag.cmake
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/cmake/modules/HandleLLVMStdlib.cmake
  llvm/cmake/modules/LLVMCheckLinkerFlag.cmake

Index: llvm/cmake/modules/LLVMCheckLinkerFlag.cmake
===
--- /dev/null
+++ llvm/cmake/modules/LLVMCheckLinkerFlag.cmake
@@ -0,0 +1,17 @@
+include(CheckLinkerFlag OPTIONAL)
+
+if (COMMAND check_linker_flag)
+  macro(llvm_check_linker_flag)
+check_linker_flag(${ARGN})
+  endmacro()
+else()
+  include(CheckCXXCompilerFlag)
+
+  # cmake builtin compatible, except we assume lang is CXX
+  function(llvm_check_linker_flag lang flag out_var)
+cmake_push_check_state()
+set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${flag}")
+check_cxx_compiler_flag("" ${out_var})
+cmake_pop_check_state()
+  endfunction()
+endif()
Index: llvm/cmake/modules/HandleLLVMStdlib.cmake
===
--- llvm/cmake/modules/HandleLLVMStdlib.cmake
+++ llvm/cmake/modules/HandleLLVMStdlib.cmake
@@ -13,12 +13,12 @@
   endfunction()
 
   include(CheckCXXCompilerFlag)
-  include(CheckLinkerFlag)
+  include(LLVMCheckLinkerFlag)
   set(LLVM_LIBCXX_USED 0)
   if(LLVM_ENABLE_LIBCXX)
 if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
   check_cxx_compiler_flag("-stdlib=libc++" CXX_COMPILER_SUPPORTS_STDLIB)
-  check_linker_flag("-stdlib=libc++" CXX_LINKER_SUPPORTS_STDLIB)
+  llvm_check_linker_flag(CXX "-stdlib=libc++" CXX_LINKER_SUPPORTS_STDLIB)
   if(CXX_COMPILER_SUPPORTS_STDLIB AND CXX_LINKER_SUPPORTS_STDLIB)
 append("-stdlib=libc++"
   CMAKE_CXX_FLAGS CMAKE_EXE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS
@@ -36,7 +36,7 @@
 if(LLVM_COMPILER_IS_GCC_COMPATIBLE)
   check_cxx_compiler_flag("-static-libstdc++"
   CXX_COMPILER_SUPPORTS_STATIC_STDLIB)
-  check_linker_flag("-static-libstdc++" CXX_LINKER_SUPPORTS_STATIC_STDLIB)
+  llvm_check_linker_flag(CXX "-static-libstdc++" CXX_LINKER_SUPPORTS_STATIC_STDLIB)
   if(CXX_COMPILER_SUPPORTS_STATIC_STDLIB AND
 CXX_LINKER_SUPPORTS_STATIC_STDLIB)
 append("-static-libstdc++"
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -888,8 +888,8 @@
 
 # lld doesn't print colored diagnostics when invoked from Ninja
 if (UNIX AND CMAKE_GENERATOR STREQUAL "Ninja")
-  include(CheckLinkerFlag)
-  check_linker_flag("-Wl,--color-diagnostics" LINKER_SUPPORTS_COLOR_DIAGNOSTICS)
+  include(LLVMCheckLinkerFlag)
+  llvm_check_linker_flag(CXX "-Wl,--color-diagnostics" LINKER_SUPPORTS_COLOR_DIAGNOSTICS)
   append_if(LINKER_SUPPORTS_COLOR_DIAGNOSTICS "-Wl,--color-diagnostics"
 CMAKE_EXE_LINKER_FLAGS CMAKE_MODULE_LINKER_FLAGS CMAKE_SHARED_LINKER_FLAGS)
 endif()
Index: llvm/cmake/modules/CheckLinkerFlag.cmake
===
--- llvm/cmake/modules/CheckLinkerFlag.cmake
+++ /dev/null
@@ -1,6 +0,0 @@
-include(CheckCXXCompilerFlag)
-
-function(check_linker_flag flag out_var)
-  set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${flag}")
-  check_cxx_compiler_flag("" ${out_var})
-endfunction()
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -236,8 +236,8 @@
   elseif(${CMAKE_SYSTEM_NAME} MATCHES "SunOS")
 # Support for ld -z discard-unused=sections was only added in
 # Solaris 11.4.
-include(CheckLinkerFlag)
-check_linker_flag("-Wl,-z,discard-unused=sections" LINKER_SUPPORTS_Z_DISCARD_UNUSED)
+include(LLVMCheckLinkerFlag)
+llvm_check_linker_flag(CXX "-Wl,-z,discard-unused=sections" LINKER_SUPPORTS_Z_DISCARD_UNUSED)
 if (LINKER_SUPPORTS_Z_DISCARD_UNUSED)
   set_property(TARGET ${target_name} APPEND_STRING PROPERTY
LINK_FLAGS " -Wl,-z,discard-unused=sections")
Index: clang/tools/driver/CMakeLists.txt
===
--- clang/tools/driver/CMakeLists.txt
+++ clang/tools/driver/CMakeLists.txt
@@ -93,7 +93,7 @@
 
 if(CLANG_ORDER_FILE AND
 (LLVM_LINKER_IS_LD64 OR LLVM_LINKER_IS_GOLD OR LLVM_LINKER_IS_LLD))
-  include(CheckLinkerFlag)
+  include(LLVMCheckLinkerFlag)
 
   if (LLVM_LINKER_IS_LD64)
 

[PATCH] D97499: [PM] Show the pass argument in pre/post-pass IR dumps

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

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

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97499

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

[PATCH] D91444: [InstCombine] Preserve !annotation metadata for memory combines.

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

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

LGTM, nice!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91444

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

[PATCH] D91417: [IRGen] Add !annotation metadata for auto-init stores.

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg accepted this revision.
thegameg added a comment.

Looks good, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91417

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

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg accepted this revision.
thegameg added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

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

[PATCH] D82213: [Remarks] Add callsite locations to inline remarks

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added subscribers: fhahn, anemet, thegameg.
thegameg added a comment.

This sounds useful indeed. @fhahn, @anemet might want to take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82213



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

[PATCH] D76916: [Darwin] Respect -fno-unroll-loops during LTO.

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg accepted this revision.
thegameg 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/D76916/new/

https://reviews.llvm.org/D76916



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

[PATCH] D74094: [IRGen] Emit lifetime intrinsics around temporary aggregate argument allocas

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:3687
 
-  args.add(EmitAnyExprToTemp(E), type);
 }

Is there any other use of `EmitAnyExprToTemp` that can benefit from this?


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

https://reviews.llvm.org/D74094



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

[PATCH] D73676: [Remarks] Extend the RemarkStreamer to support other emitters

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
thegameg marked 2 inline comments as done.
Closed by commit rG7531a5039fd7: [Remarks] Extend the RemarkStreamer to support 
other emitters (authored by thegameg).

Changed prior to commit:
  https://reviews.llvm.org/D73676?vs=241324=242475#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73676

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  llvm/docs/Remarks.rst
  llvm/include/llvm/Analysis/OptimizationRemarkEmitter.h
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineOptimizationRemarkEmitter.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/IR/LLVMRemarkStreamer.h
  llvm/include/llvm/IR/RemarkStreamer.h
  llvm/include/llvm/LTO/LTO.h
  llvm/include/llvm/Remarks/RemarkStreamer.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/IR/RemarkStreamer.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/lib/Remarks/CMakeLists.txt
  llvm/lib/Remarks/RemarkStreamer.cpp
  llvm/tools/llc/llc.cpp
  llvm/tools/opt/opt.cpp

Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -29,10 +29,10 @@
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/LLVMRemarkStreamer.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/LegacyPassNameParser.h"
 #include "llvm/IR/Module.h"
-#include "llvm/IR/RemarkStreamer.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/InitializePasses.h"
@@ -583,9 +583,9 @@
 Context.enableDebugTypeODRUniquing();
 
   Expected> RemarksFileOrErr =
-  setupOptimizationRemarks(Context, RemarksFilename, RemarksPasses,
-   RemarksFormat, RemarksWithHotness,
-   RemarksHotnessThreshold);
+  setupLLVMOptimizationRemarks(Context, RemarksFilename, RemarksPasses,
+   RemarksFormat, RemarksWithHotness,
+   RemarksHotnessThreshold);
   if (Error E = RemarksFileOrErr.takeError()) {
 errs() << toString(std::move(E)) << '\n';
 return 1;
Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -29,9 +29,9 @@
 #include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/LLVMRemarkStreamer.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Module.h"
-#include "llvm/IR/RemarkStreamer.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/InitializePasses.h"
@@ -334,9 +334,9 @@
   Context.setInlineAsmDiagnosticHandler(InlineAsmDiagHandler, );
 
   Expected> RemarksFileOrErr =
-  setupOptimizationRemarks(Context, RemarksFilename, RemarksPasses,
-   RemarksFormat, RemarksWithHotness,
-   RemarksHotnessThreshold);
+  setupLLVMOptimizationRemarks(Context, RemarksFilename, RemarksPasses,
+   RemarksFormat, RemarksWithHotness,
+   RemarksHotnessThreshold);
   if (Error E = RemarksFileOrErr.takeError()) {
 WithColor::error(errs(), argv[0]) << toString(std::move(E)) << '\n';
 return 1;
Index: llvm/lib/Remarks/RemarkStreamer.cpp
===
--- /dev/null
+++ llvm/lib/Remarks/RemarkStreamer.cpp
@@ -0,0 +1,72 @@
+//===- llvm/Remarks/RemarkStreamer.cpp - Remark Streamer -*- C++ *-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file contains the implementation of the main remark streamer.
+//
+//===--===//
+
+#include "llvm/Remarks/RemarkStreamer.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace llvm;
+using namespace llvm::remarks;
+
+static cl::opt EnableRemarksSection(
+"remarks-section",
+cl::desc(
+"Emit a section containing remark diagnostics metadata. By default, "
+"this is enabled for the following formats: yaml-strtab, bitstream."),
+cl::init(cl::BOU_UNSET), cl::Hidden);
+
+RemarkStreamer::RemarkStreamer(
+std::unique_ptr RemarkSerializer,
+Optional FilenameIn)
+: PassFilter(), 

[PATCH] D73676: [Remarks] Extend the RemarkStreamer to support other emitters

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg created this revision.
thegameg added reviewers: paquette, anemet, JDevlieghere, hfinkel, fhahn.
thegameg added projects: LLVM, clang.
Herald added subscribers: dang, dexonsmith, steven_wu, hiraditya, mgorny, 
mehdi_amini.

This extends the RemarkStreamer to allow for other emitters (e.g. frontends, 
SIL, etc.) to emit remarks through a common interface.

See changes in llvm/docs/Remarks.rst for motivation and design choices.


https://reviews.llvm.org/D73676

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  llvm/docs/Remarks.rst
  llvm/include/llvm/Analysis/OptimizationRemarkEmitter.h
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineOptimizationRemarkEmitter.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/IR/LLVMRemarkStreamer.h
  llvm/include/llvm/IR/RemarkStreamer.h
  llvm/include/llvm/LTO/LTO.h
  llvm/include/llvm/Remarks/RemarkStreamer.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/IR/LLVMRemarkStreamer.cpp
  llvm/lib/IR/RemarkStreamer.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/lib/Remarks/CMakeLists.txt
  llvm/lib/Remarks/RemarkStreamer.cpp
  llvm/tools/llc/llc.cpp
  llvm/tools/opt/opt.cpp

Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -29,10 +29,10 @@
 #include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/LLVMRemarkStreamer.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/LegacyPassNameParser.h"
 #include "llvm/IR/Module.h"
-#include "llvm/IR/RemarkStreamer.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/InitializePasses.h"
@@ -583,9 +583,9 @@
 Context.enableDebugTypeODRUniquing();
 
   Expected> RemarksFileOrErr =
-  setupOptimizationRemarks(Context, RemarksFilename, RemarksPasses,
-   RemarksFormat, RemarksWithHotness,
-   RemarksHotnessThreshold);
+  setupLLVMOptimizationRemarks(Context, RemarksFilename, RemarksPasses,
+   RemarksFormat, RemarksWithHotness,
+   RemarksHotnessThreshold);
   if (Error E = RemarksFileOrErr.takeError()) {
 errs() << toString(std::move(E)) << '\n';
 return 1;
Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -29,9 +29,9 @@
 #include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/IRPrintingPasses.h"
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/LLVMRemarkStreamer.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Module.h"
-#include "llvm/IR/RemarkStreamer.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/InitializePasses.h"
@@ -334,9 +334,9 @@
   Context.setInlineAsmDiagnosticHandler(InlineAsmDiagHandler, );
 
   Expected> RemarksFileOrErr =
-  setupOptimizationRemarks(Context, RemarksFilename, RemarksPasses,
-   RemarksFormat, RemarksWithHotness,
-   RemarksHotnessThreshold);
+  setupLLVMOptimizationRemarks(Context, RemarksFilename, RemarksPasses,
+   RemarksFormat, RemarksWithHotness,
+   RemarksHotnessThreshold);
   if (Error E = RemarksFileOrErr.takeError()) {
 WithColor::error(errs(), argv[0]) << toString(std::move(E)) << '\n';
 return 1;
Index: llvm/lib/Remarks/RemarkStreamer.cpp
===
--- /dev/null
+++ llvm/lib/Remarks/RemarkStreamer.cpp
@@ -0,0 +1,72 @@
+//===- llvm/Remarks/RemarkStreamer.cpp - Remark Streamer -*- C++ *-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This file contains the implementation of the main remark streamer.
+//
+//===--===//
+
+#include "llvm/Remarks/RemarkStreamer.h"
+#include "llvm/Support/CommandLine.h"
+
+using namespace llvm;
+using namespace llvm::remarks;
+
+static cl::opt EnableRemarksSection(
+"remarks-section",
+cl::desc(
+"Emit a section containing remark diagnostics metadata. By default, "
+"this is enabled for the following formats: yaml-strtab, bitstream."),
+cl::init(cl::BOU_UNSET), cl::Hidden);
+
+RemarkStreamer::RemarkStreamer(
+std::unique_ptr RemarkSerializer,
+Optional FilenameIn)
+: 

[PATCH] D73495: [CodeGen] Attach no-builtin attributes to function definitions with no Decl

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4e799ada5860: [CodeGen] Attach no-builtin attributes to 
function definitions with no Decl (authored by thegameg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73495

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenCXX/global-init.cpp

Index: clang/test/CodeGenCXX/global-init.cpp
===
--- clang/test/CodeGenCXX/global-init.cpp
+++ clang/test/CodeGenCXX/global-init.cpp
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm %s -o - |FileCheck -check-prefix CHECK-NOEXC %s
 // RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -mframe-pointer=non-leaf %s -o - \
 // RUN:   | FileCheck -check-prefix CHECK-FP %s
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm %s -o - -fno-builtin \
+// RUN:   | FileCheck -check-prefix CHECK-NOBUILTIN %s
 
 struct A {
   A();
@@ -202,9 +204,12 @@
 
 // rdar://problem/8090834: this should be nounwind
 // CHECK-NOEXC: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUW:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
-
 // CHECK-NOEXC: attributes [[NUW]] = { noinline nounwind{{.*}} }
 
+// Make sure we mark global initializers with the no-builtins attribute.
+// CHECK-NOBUILTIN: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUW:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
+// CHECK-NOBUILTIN: attributes [[NUW]] = { noinline nounwind{{.*}}"no-builtins"{{.*}} }
+
 // PR21811: attach the appropriate attribute to the global init function
 // CHECK-FP: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUX:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
 // CHECK-FP: attributes [[NUX]] = { noinline nounwind {{.*}}"frame-pointer"="non-leaf"{{.*}} }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1832,6 +1832,42 @@
   F.addAttributes(llvm::AttributeList::FunctionIndex, FuncAttrs);
 }
 
+static void addNoBuiltinAttributes(llvm::AttrBuilder ,
+   const LangOptions ,
+   const NoBuiltinAttr *NBA = nullptr) {
+  auto AddNoBuiltinAttr = [](StringRef BuiltinName) {
+SmallString<32> AttributeName;
+AttributeName += "no-builtin-";
+AttributeName += BuiltinName;
+FuncAttrs.addAttribute(AttributeName);
+  };
+
+  // First, handle the language options passed through -fno-builtin[-]
+  if (LangOpts.NoBuiltin) {
+// -fno-builtin disables them all.
+FuncAttrs.addAttribute("no-builtins");
+return;
+  }
+
+  // Then, add attributes for builtins specified through -fno-builtin-.
+  llvm::for_each(LangOpts.NoBuiltinFuncs, AddNoBuiltinAttr);
+
+  // Now, let's check the __attribute__((no_builtin("...")) attribute added to
+  // the source.
+  if (!NBA)
+return;
+
+  // If there is a wildcard in the builtin names specified through the
+  // attribute, disable them all.
+  if (llvm::is_contained(NBA->builtinNames(), "*")) {
+FuncAttrs.addAttribute("no-builtins");
+return;
+  }
+
+  // And last, add the rest of the builtin names.
+  llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
+}
+
 void CodeGenModule::ConstructAttributeList(
 StringRef Name, const CGFunctionInfo , CGCalleeInfo CalleeInfo,
 llvm::AttributeList , unsigned , bool AttrOnCallSite) {
@@ -1850,6 +1886,8 @@
   const Decl *TargetDecl = CalleeInfo.getCalleeDecl().getDecl();
 
   bool HasOptnone = false;
+  // The NoBuiltinAttr attached to a TargetDecl (only allowed on FunctionDecls).
+  const NoBuiltinAttr *NBA = nullptr;
   // FIXME: handle sseregparm someday...
   if (TargetDecl) {
 if (TargetDecl->hasAttr())
@@ -1875,22 +1913,7 @@
   if (!(AttrOnCallSite && IsVirtualCall)) {
 if (Fn->isNoReturn())
   FuncAttrs.addAttribute(llvm::Attribute::NoReturn);
-
-const auto *NBA = Fn->getAttr();
-bool HasWildcard = NBA && llvm::is_contained(NBA->builtinNames(), "*");
-if (getLangOpts().NoBuiltin || HasWildcard)
-  FuncAttrs.addAttribute("no-builtins");
-else {
-  auto AddNoBuiltinAttr = [](StringRef BuiltinName) {
-SmallString<32> AttributeName;
-AttributeName += "no-builtin-";
-AttributeName += BuiltinName;
-FuncAttrs.addAttribute(AttributeName);
-  };
-  llvm::for_each(getLangOpts().NoBuiltinFuncs, AddNoBuiltinAttr);
-  if (NBA)
-llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
-}
+NBA = Fn->getAttr();
   }
 }
 
@@ -1925,6 +1948,14 @@
 }
   }
 
+  // Attach "no-builtins" attributes to:
+  // * call sites: both `nobuiltin` and "no-builtins" or 

[PATCH] D73495: [CodeGen] Attach no-builtin attributes to function definitions with no Decl

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg updated this revision to Diff 240914.
thegameg added a comment.

Add the attribute to all TargetDecls.


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

https://reviews.llvm.org/D73495

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenCXX/global-init.cpp

Index: clang/test/CodeGenCXX/global-init.cpp
===
--- clang/test/CodeGenCXX/global-init.cpp
+++ clang/test/CodeGenCXX/global-init.cpp
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm %s -o - |FileCheck -check-prefix CHECK-NOEXC %s
 // RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -mframe-pointer=non-leaf %s -o - \
 // RUN:   | FileCheck -check-prefix CHECK-FP %s
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm %s -o - -fno-builtin \
+// RUN:   | FileCheck -check-prefix CHECK-NOBUILTIN %s
 
 struct A {
   A();
@@ -202,9 +204,12 @@
 
 // rdar://problem/8090834: this should be nounwind
 // CHECK-NOEXC: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUW:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
-
 // CHECK-NOEXC: attributes [[NUW]] = { noinline nounwind{{.*}} }
 
+// Make sure we mark global initializers with the no-builtins attribute.
+// CHECK-NOBUILTIN: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUW:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
+// CHECK-NOBUILTIN: attributes [[NUW]] = { noinline nounwind{{.*}}"no-builtins"{{.*}} }
+
 // PR21811: attach the appropriate attribute to the global init function
 // CHECK-FP: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUX:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
 // CHECK-FP: attributes [[NUX]] = { noinline nounwind {{.*}}"frame-pointer"="non-leaf"{{.*}} }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1832,6 +1832,42 @@
   F.addAttributes(llvm::AttributeList::FunctionIndex, FuncAttrs);
 }
 
+static void addNoBuiltinAttributes(llvm::AttrBuilder ,
+   const LangOptions ,
+   const NoBuiltinAttr *NBA = nullptr) {
+  auto AddNoBuiltinAttr = [](StringRef BuiltinName) {
+SmallString<32> AttributeName;
+AttributeName += "no-builtin-";
+AttributeName += BuiltinName;
+FuncAttrs.addAttribute(AttributeName);
+  };
+
+  // First, handle the language options passed through -fno-builtin[-]
+  if (LangOpts.NoBuiltin) {
+// -fno-builtin disables them all.
+FuncAttrs.addAttribute("no-builtins");
+return;
+  }
+
+  // Then, add attributes for builtins specified through -fno-builtin-.
+  llvm::for_each(LangOpts.NoBuiltinFuncs, AddNoBuiltinAttr);
+
+  // Now, let's check the __attribute__((no_builtin("...")) attribute added to
+  // the source.
+  if (!NBA)
+return;
+
+  // If there is a wildcard in the builtin names specified through the
+  // attribute, disable them all.
+  if (llvm::is_contained(NBA->builtinNames(), "*")) {
+FuncAttrs.addAttribute("no-builtins");
+return;
+  }
+
+  // And last, add the rest of the builtin names.
+  llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
+}
+
 void CodeGenModule::ConstructAttributeList(
 StringRef Name, const CGFunctionInfo , CGCalleeInfo CalleeInfo,
 llvm::AttributeList , unsigned , bool AttrOnCallSite) {
@@ -1850,6 +1886,8 @@
   const Decl *TargetDecl = CalleeInfo.getCalleeDecl().getDecl();
 
   bool HasOptnone = false;
+  // The NoBuiltinAttr attached to a TargetDecl (only allowed on FunctionDecls).
+  const NoBuiltinAttr *NBA = nullptr;
   // FIXME: handle sseregparm someday...
   if (TargetDecl) {
 if (TargetDecl->hasAttr())
@@ -1875,22 +1913,7 @@
   if (!(AttrOnCallSite && IsVirtualCall)) {
 if (Fn->isNoReturn())
   FuncAttrs.addAttribute(llvm::Attribute::NoReturn);
-
-const auto *NBA = Fn->getAttr();
-bool HasWildcard = NBA && llvm::is_contained(NBA->builtinNames(), "*");
-if (getLangOpts().NoBuiltin || HasWildcard)
-  FuncAttrs.addAttribute("no-builtins");
-else {
-  auto AddNoBuiltinAttr = [](StringRef BuiltinName) {
-SmallString<32> AttributeName;
-AttributeName += "no-builtin-";
-AttributeName += BuiltinName;
-FuncAttrs.addAttribute(AttributeName);
-  };
-  llvm::for_each(getLangOpts().NoBuiltinFuncs, AddNoBuiltinAttr);
-  if (NBA)
-llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
-}
+NBA = Fn->getAttr();
   }
 }
 
@@ -1925,6 +1948,14 @@
 }
   }
 
+  // Attach "no-builtins" attributes to:
+  // * call sites: both `nobuiltin` and "no-builtins" or "no-builtin-".
+  // * definitions: "no-builtins" or "no-builtin-" only.
+  // The attributes can come from:
+  // * LangOpts: -ffreestanding, 

[PATCH] D73495: [CodeGen] Attach no-builtin attributes to function definitions with no Decl

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGCall.cpp:1917
 const auto *NBA = Fn->getAttr();
-bool HasWildcard = NBA && llvm::is_contained(NBA->builtinNames(), "*");
-if (getLangOpts().NoBuiltin || HasWildcard)
-  FuncAttrs.addAttribute("no-builtins");
-else {
-  auto AddNoBuiltinAttr = [](StringRef BuiltinName) {
-SmallString<32> AttributeName;
-AttributeName += "no-builtin-";
-AttributeName += BuiltinName;
-FuncAttrs.addAttribute(AttributeName);
-  };
-  llvm::for_each(getLangOpts().NoBuiltinFuncs, AddNoBuiltinAttr);
-  if (NBA)
-llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
-}
+addNonCallSiteNoBuiltinAttributes(FuncAttrs, getLangOpts(), NBA);
   }

efriedma wrote:
> thegameg wrote:
> > efriedma wrote:
> > > What happens if we have a TargetDecl that isn't a FunctionDecl?  (I think 
> > > this happens in some cases, but not completely sure which, exactly.)
> > It looks like that can be triggered by indirect calls:
> > 
> > ```
> > typedef void T(void);
> > void test3(T f) {
> >   f();
> > }
> > ```
> > 
> > Since this adds the attribute to definitions and not to call sites, we 
> > should never need that.
> > 
> > This patch is for the case where `CreateGlobalInitOrDestructFunction` ends 
> > up re-using the same function to attach the attributes.
> > 
> > I'll update the description to make it more clear.
> Are you sure that's the only case where the TargetDecl isn't a FunctionDecl?  
> I'm afraid there might be some weird case where the TargetDecl defines 
> something like a function, but isn't technically a FunctionDecl. Maybe an 
> ObjC method or something like that.  And then we end up in a situation where 
> addNonCallSiteNoBuiltinAttributes is never called.
Right, it looks like in the test suite this triggers on:

* BlockDecl
* ObjCMethodDecl
* CapturedDecl

I think we should call `addNonCallSiteNoBuiltinAttributes` for these too.


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

https://reviews.llvm.org/D73495



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

[PATCH] D73495: [CodeGen] Attach no-builtin attributes to functions with no Decl

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGCall.cpp:1917
 const auto *NBA = Fn->getAttr();
-bool HasWildcard = NBA && llvm::is_contained(NBA->builtinNames(), "*");
-if (getLangOpts().NoBuiltin || HasWildcard)
-  FuncAttrs.addAttribute("no-builtins");
-else {
-  auto AddNoBuiltinAttr = [](StringRef BuiltinName) {
-SmallString<32> AttributeName;
-AttributeName += "no-builtin-";
-AttributeName += BuiltinName;
-FuncAttrs.addAttribute(AttributeName);
-  };
-  llvm::for_each(getLangOpts().NoBuiltinFuncs, AddNoBuiltinAttr);
-  if (NBA)
-llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
-}
+addNonCallSiteNoBuiltinAttributes(FuncAttrs, getLangOpts(), NBA);
   }

efriedma wrote:
> What happens if we have a TargetDecl that isn't a FunctionDecl?  (I think 
> this happens in some cases, but not completely sure which, exactly.)
It looks like that can be triggered by indirect calls:

```
typedef void T(void);
void test3(T f) {
  f();
}
```

Since this adds the attribute to definitions and not to call sites, we should 
never need that.

This patch is for the case where `CreateGlobalInitOrDestructFunction` ends up 
re-using the same function to attach the attributes.

I'll update the description to make it more clear.


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

https://reviews.llvm.org/D73495



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

[PATCH] D73495: [CodeGen] Attach no-builtin attributes to functions with no Decl

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg created this revision.
thegameg added reviewers: gchatelet, tejohnson, aaron.ballman, efriedma.
thegameg added a project: clang.

When using -fno-builtin[-], we don't attach the IR attributes to 
functions with no Decl.

This results in projects using -fno-builtin or -ffreestanding to start seeing 
symbols like _memset_pattern16.

The fix changes the behavior to always add the attribute if LangOptions 
requests it.


https://reviews.llvm.org/D73495

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenCXX/global-init.cpp

Index: clang/test/CodeGenCXX/global-init.cpp
===
--- clang/test/CodeGenCXX/global-init.cpp
+++ clang/test/CodeGenCXX/global-init.cpp
@@ -2,6 +2,8 @@
 // RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm %s -o - |FileCheck -check-prefix CHECK-NOEXC %s
 // RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -mframe-pointer=non-leaf %s -o - \
 // RUN:   | FileCheck -check-prefix CHECK-FP %s
+// RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm %s -o - -fno-builtin \
+// RUN:   | FileCheck -check-prefix CHECK-NOBUILTIN %s
 
 struct A {
   A();
@@ -202,9 +204,12 @@
 
 // rdar://problem/8090834: this should be nounwind
 // CHECK-NOEXC: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUW:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
-
 // CHECK-NOEXC: attributes [[NUW]] = { noinline nounwind{{.*}} }
 
+// Make sure we mark global initializers with the no-builtins attribute.
+// CHECK-NOBUILTIN: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUW:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
+// CHECK-NOBUILTIN: attributes [[NUW]] = { noinline nounwind{{.*}}"no-builtins"{{.*}} }
+
 // PR21811: attach the appropriate attribute to the global init function
 // CHECK-FP: define internal void @_GLOBAL__sub_I_global_init.cpp() [[NUX:#[0-9]+]] section "__TEXT,__StaticInit,regular,pure_instructions" {
 // CHECK-FP: attributes [[NUX]] = { noinline nounwind {{.*}}"frame-pointer"="non-leaf"{{.*}} }
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1832,6 +1832,43 @@
   F.addAttributes(llvm::AttributeList::FunctionIndex, FuncAttrs);
 }
 
+static void
+addNonCallSiteNoBuiltinAttributes(llvm::AttrBuilder ,
+  const LangOptions ,
+  const NoBuiltinAttr *NBA = nullptr) {
+  auto AddNoBuiltinAttr = [](StringRef BuiltinName) {
+SmallString<32> AttributeName;
+AttributeName += "no-builtin-";
+AttributeName += BuiltinName;
+FuncAttrs.addAttribute(AttributeName);
+  };
+
+  // First, handle the language options passed through -fno-builtin[-]
+  if (LangOpts.NoBuiltin) {
+// -fno-builtin disables them all.
+FuncAttrs.addAttribute("no-builtins");
+return;
+  }
+
+  // Then, add attributes for builtins specified through -fno-builtin-.
+  llvm::for_each(LangOpts.NoBuiltinFuncs, AddNoBuiltinAttr);
+
+  // Now, let's check the __attribute__((no_builtin("...")) attribute added to
+  // the source.
+  if (!NBA)
+return;
+
+  // If there is a wildcard in the builtin names specified through the
+  // attribute, disable them all.
+  if (llvm::is_contained(NBA->builtinNames(), "*")) {
+FuncAttrs.addAttribute("no-builtins");
+return;
+  }
+
+  // And last, add the rest of the builtin names.
+  llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
+}
+
 void CodeGenModule::ConstructAttributeList(
 StringRef Name, const CGFunctionInfo , CGCalleeInfo CalleeInfo,
 llvm::AttributeList , unsigned , bool AttrOnCallSite) {
@@ -1877,20 +1914,7 @@
   FuncAttrs.addAttribute(llvm::Attribute::NoReturn);
 
 const auto *NBA = Fn->getAttr();
-bool HasWildcard = NBA && llvm::is_contained(NBA->builtinNames(), "*");
-if (getLangOpts().NoBuiltin || HasWildcard)
-  FuncAttrs.addAttribute("no-builtins");
-else {
-  auto AddNoBuiltinAttr = [](StringRef BuiltinName) {
-SmallString<32> AttributeName;
-AttributeName += "no-builtin-";
-AttributeName += BuiltinName;
-FuncAttrs.addAttribute(AttributeName);
-  };
-  llvm::for_each(getLangOpts().NoBuiltinFuncs, AddNoBuiltinAttr);
-  if (NBA)
-llvm::for_each(NBA->builtinNames(), AddNoBuiltinAttr);
-}
+addNonCallSiteNoBuiltinAttributes(FuncAttrs, getLangOpts(), NBA);
   }
 }
 
@@ -1923,6 +1947,10 @@
   FuncAttrs.addAllocSizeAttr(AllocSize->getElemSizeParam().getLLVMIndex(),
  NumElemsParam);
 }
+  } else {
+// If there is no TargetDecl, we still want to attach the attributes to
+// functions that are compiler-generated, like static initializers.
+addNonCallSiteNoBuiltinAttributes(FuncAttrs, getLangOpts());
   }

[PATCH] D71301: [clang][IFS] Prevent Clang-IFS from Leaking symbols from inside a block.

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

In D71301#1832801 , @plotfi wrote:

> So, the culprit appears to be D69825 . The 
> way InterfaceStubs assembles the pipeline appears to trigger an asan bug. For 
> now I will alter the tests to get the bots green, but once I get my bugzilla 
> access sorted I intend to file a bug. -PL
>
> In D71301#1826928 , @thegameg wrote:
>
> > Hi @plotfi, this seems to cause failures with ASAN and UBSAN on green 
> > dragon: 
> > http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6886/consoleFull
> >
> > The following tests are failing:
> >
> >   Clang :: InterfaceStubs/driver-test.c
> >   Clang :: InterfaceStubs/driver-test2.c
> >   
>


Great, thanks for taking a look!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71301



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

[PATCH] D71301: [clang][IFS] Prevent Clang-IFS from Leaking symbols from inside a block.

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

Hi @plotfi, this seems to cause failures with ASAN and UBSAN on green dragon: 
http://green.lab.llvm.org/green/job/clang-stage2-cmake-RgSan/6886/consoleFull

The following tests are failing:

  Clang :: InterfaceStubs/driver-test.c
  Clang :: InterfaceStubs/driver-test2.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71301



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

[PATCH] D71675: [Remarks][Driver] Run dsymutil when remarks are enabled

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd79b11fefb8e: [Remarks][Driver] Run dsymutil when remarks 
are enabled (authored by thegameg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71675

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/darwin-opt-record.c


Index: clang/test/Driver/darwin-opt-record.c
===
--- clang/test/Driver/darwin-opt-record.c
+++ clang/test/Driver/darwin-opt-record.c
@@ -2,6 +2,8 @@
 
 // RUN: %clang -target x86_64-apple-darwin10 -### -c -o FOO 
-fsave-optimization-record -arch x86_64 -arch x86_64h %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-MULTIPLE-ARCH
 // RUN: %clang -target x86_64-apple-darwin10 -### -c -o FOO 
-foptimization-record-file=tmp -arch x86_64 -arch x86_64h %s 2>&1 | FileCheck 
%s --check-prefix=CHECK-MULTIPLE-ARCH-ERROR
+// RUN: %clang -target x86_64-apple-darwin10 -### -o FOO 
-fsave-optimization-record %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-DSYMUTIL-NO-G
+// RUN: %clang -target x86_64-apple-darwin10 -### -o FOO -g0 
-fsave-optimization-record %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-DSYMUTIL-G0
 //
 // CHECK-MULTIPLE-ARCH: "-cc1"
 // CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"
@@ -9,3 +11,14 @@
 // CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64h.opt.yaml"
 //
 // CHECK-MULTIPLE-ARCH-ERROR: cannot use '-foptimization-record-file' output 
with multiple -arch options
+//
+// CHECK-DSYMUTIL-NO-G: "-cc1"
+// CHECK-DSYMUTIL-NO-G: ld
+// CHECK-DSYMUTIL-NO-G: dsymutil
+//
+// Even in the presence of -g0, -fsave-optimization-record implies
+// -gline-tables-only and would need -fno-save-optimization-record to
+// completely disable it.
+// CHECK-DSYMUTIL-G0: "-cc1"
+// CHECK-DSYMUTIL-G0: ld
+// CHECK-DSYMUTIL-G0: dsymutil
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1990,8 +1990,9 @@
 
 // Handle debug info queries.
 Arg *A = Args.getLastArg(options::OPT_g_Group);
-if (A && !A->getOption().matches(options::OPT_g0) &&
-!A->getOption().matches(options::OPT_gstabs) &&
+bool enablesDebugInfo = A && !A->getOption().matches(options::OPT_g0) &&
+!A->getOption().matches(options::OPT_gstabs);
+if ((enablesDebugInfo || willEmitRemarks(Args)) &&
 ContainsCompileOrAssembleAction(Actions.back())) {
 
   // Add a 'dsymutil' step if necessary, when debug info is enabled and we


Index: clang/test/Driver/darwin-opt-record.c
===
--- clang/test/Driver/darwin-opt-record.c
+++ clang/test/Driver/darwin-opt-record.c
@@ -2,6 +2,8 @@
 
 // RUN: %clang -target x86_64-apple-darwin10 -### -c -o FOO -fsave-optimization-record -arch x86_64 -arch x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH
 // RUN: %clang -target x86_64-apple-darwin10 -### -c -o FOO -foptimization-record-file=tmp -arch x86_64 -arch x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH-ERROR
+// RUN: %clang -target x86_64-apple-darwin10 -### -o FOO -fsave-optimization-record %s 2>&1 | FileCheck %s --check-prefix=CHECK-DSYMUTIL-NO-G
+// RUN: %clang -target x86_64-apple-darwin10 -### -o FOO -g0 -fsave-optimization-record %s 2>&1 | FileCheck %s --check-prefix=CHECK-DSYMUTIL-G0
 //
 // CHECK-MULTIPLE-ARCH: "-cc1"
 // CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"
@@ -9,3 +11,14 @@
 // CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64h.opt.yaml"
 //
 // CHECK-MULTIPLE-ARCH-ERROR: cannot use '-foptimization-record-file' output with multiple -arch options
+//
+// CHECK-DSYMUTIL-NO-G: "-cc1"
+// CHECK-DSYMUTIL-NO-G: ld
+// CHECK-DSYMUTIL-NO-G: dsymutil
+//
+// Even in the presence of -g0, -fsave-optimization-record implies
+// -gline-tables-only and would need -fno-save-optimization-record to
+// completely disable it.
+// CHECK-DSYMUTIL-G0: "-cc1"
+// CHECK-DSYMUTIL-G0: ld
+// CHECK-DSYMUTIL-G0: dsymutil
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1990,8 +1990,9 @@
 
 // Handle debug info queries.
 Arg *A = Args.getLastArg(options::OPT_g_Group);
-if (A && !A->getOption().matches(options::OPT_g0) &&
-!A->getOption().matches(options::OPT_gstabs) &&
+bool enablesDebugInfo = A && !A->getOption().matches(options::OPT_g0) &&
+!A->getOption().matches(options::OPT_gstabs);
+if ((enablesDebugInfo || willEmitRemarks(Args)) &&
 ContainsCompileOrAssembleAction(Actions.back())) {
 
   // Add a 'dsymutil' step if necessary, when debug info is enabled and we
___
cfe-commits mailing 

[PATCH] D71675: [Remarks][Driver] Run dsymutil when remarks are enabled

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/darwin-opt-record.c:21
+// -gline-tables-only and would need -fno-save-optimization-record to
+// completely disable it.
+// CHECK-DSYMUTIL-G0: "-cc1"

The other choice would be honoring `-g0` and:

* not passing `debug-info-kind=` to `cc1`
* not running `dsymutil`
* possibly emit a warning/error that the options are incompatible


Repository:
  rC Clang

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

https://reviews.llvm.org/D71675



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

[PATCH] D71675: [Remarks][Driver] Run dsymutil when remarks are enabled

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg created this revision.
thegameg added reviewers: friss, JDevlieghere, aprantl.
thegameg added projects: clang, debug-info.

When clang is invoked with a source file without -c or -S, it creates a cc1 
job, a linker job and if debug info is requested, a dsymutil job. In case of 
remarks, we should also create a dsymutil job to avoid losing the remarks that 
will be generated in a tempdir that gets removed.


Repository:
  rC Clang

https://reviews.llvm.org/D71675

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/darwin-opt-record.c


Index: clang/test/Driver/darwin-opt-record.c
===
--- clang/test/Driver/darwin-opt-record.c
+++ clang/test/Driver/darwin-opt-record.c
@@ -2,6 +2,8 @@
 
 // RUN: %clang -target x86_64-apple-darwin10 -### -c -o FOO 
-fsave-optimization-record -arch x86_64 -arch x86_64h %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-MULTIPLE-ARCH
 // RUN: %clang -target x86_64-apple-darwin10 -### -c -o FOO 
-foptimization-record-file=tmp -arch x86_64 -arch x86_64h %s 2>&1 | FileCheck 
%s --check-prefix=CHECK-MULTIPLE-ARCH-ERROR
+// RUN: %clang -target x86_64-apple-darwin10 -### -o FOO 
-fsave-optimization-record %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-DSYMUTIL-NO-G
+// RUN: %clang -target x86_64-apple-darwin10 -### -o FOO -g0 
-fsave-optimization-record %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-DSYMUTIL-G0
 //
 // CHECK-MULTIPLE-ARCH: "-cc1"
 // CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"
@@ -9,3 +11,14 @@
 // CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64h.opt.yaml"
 //
 // CHECK-MULTIPLE-ARCH-ERROR: cannot use '-foptimization-record-file' output 
with multiple -arch options
+//
+// CHECK-DSYMUTIL-NO-G: "-cc1"
+// CHECK-DSYMUTIL-NO-G: ld
+// CHECK-DSYMUTIL-NO-G: dsymutil
+//
+// Even in the presence of -g0, -fsave-optimization-record implies
+// -gline-tables-only and would need -fno-save-optimization-record to
+// completely disable it.
+// CHECK-DSYMUTIL-G0: "-cc1"
+// CHECK-DSYMUTIL-G0: ld
+// CHECK-DSYMUTIL-G0: dsymutil
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1990,8 +1990,9 @@
 
 // Handle debug info queries.
 Arg *A = Args.getLastArg(options::OPT_g_Group);
-if (A && !A->getOption().matches(options::OPT_g0) &&
-!A->getOption().matches(options::OPT_gstabs) &&
+bool enablesDebugInfo = A && !A->getOption().matches(options::OPT_g0) &&
+!A->getOption().matches(options::OPT_gstabs);
+if ((enablesDebugInfo || willEmitRemarks(Args)) &&
 ContainsCompileOrAssembleAction(Actions.back())) {
 
   // Add a 'dsymutil' step if necessary, when debug info is enabled and we


Index: clang/test/Driver/darwin-opt-record.c
===
--- clang/test/Driver/darwin-opt-record.c
+++ clang/test/Driver/darwin-opt-record.c
@@ -2,6 +2,8 @@
 
 // RUN: %clang -target x86_64-apple-darwin10 -### -c -o FOO -fsave-optimization-record -arch x86_64 -arch x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH
 // RUN: %clang -target x86_64-apple-darwin10 -### -c -o FOO -foptimization-record-file=tmp -arch x86_64 -arch x86_64h %s 2>&1 | FileCheck %s --check-prefix=CHECK-MULTIPLE-ARCH-ERROR
+// RUN: %clang -target x86_64-apple-darwin10 -### -o FOO -fsave-optimization-record %s 2>&1 | FileCheck %s --check-prefix=CHECK-DSYMUTIL-NO-G
+// RUN: %clang -target x86_64-apple-darwin10 -### -o FOO -g0 -fsave-optimization-record %s 2>&1 | FileCheck %s --check-prefix=CHECK-DSYMUTIL-G0
 //
 // CHECK-MULTIPLE-ARCH: "-cc1"
 // CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64.opt.yaml"
@@ -9,3 +11,14 @@
 // CHECK-MULTIPLE-ARCH: "-opt-record-file" "FOO-x86_64h.opt.yaml"
 //
 // CHECK-MULTIPLE-ARCH-ERROR: cannot use '-foptimization-record-file' output with multiple -arch options
+//
+// CHECK-DSYMUTIL-NO-G: "-cc1"
+// CHECK-DSYMUTIL-NO-G: ld
+// CHECK-DSYMUTIL-NO-G: dsymutil
+//
+// Even in the presence of -g0, -fsave-optimization-record implies
+// -gline-tables-only and would need -fno-save-optimization-record to
+// completely disable it.
+// CHECK-DSYMUTIL-G0: "-cc1"
+// CHECK-DSYMUTIL-G0: ld
+// CHECK-DSYMUTIL-G0: dsymutil
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -1990,8 +1990,9 @@
 
 // Handle debug info queries.
 Arg *A = Args.getLastArg(options::OPT_g_Group);
-if (A && !A->getOption().matches(options::OPT_g0) &&
-!A->getOption().matches(options::OPT_gstabs) &&
+bool enablesDebugInfo = A && !A->getOption().matches(options::OPT_g0) &&
+!A->getOption().matches(options::OPT_gstabs);
+if ((enablesDebugInfo || willEmitRemarks(Args)) &&
 ContainsCompileOrAssembleAction(Actions.back())) {

[PATCH] D71325: [Remarks][Driver] Ask for line tables when remarks are enabled

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG60590b149b33: [Remarks][Driver] Ask for line tables when 
remarks are enabled (authored by thegameg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71325

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -64,6 +64,12 @@
 // RUN: %clang -### -c -g %s -target arm64-apple-tvos9.0 2>&1 \
 // RUN: | FileCheck -check-prefix=G_STANDALONE \
 // RUN: -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=GLTO_ONLY %s
+// RUN: %clang -### -c -g -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=G_STANDALONE %s
 
 // FreeBSD.
 // RUN: %clang -### -c -g %s -target x86_64-pc-freebsd11.0 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3717,6 +3717,11 @@
   // Adjust the debug info kind for the given toolchain.
   TC.adjustDebugInfoKind(DebugInfoKind, Args);
 
+  // When emitting remarks, we need at least debug lines in the output.
+  if (shouldEmitRemarks(Args) &&
+  DebugInfoKind <= codegenoptions::DebugDirectivesOnly)
+DebugInfoKind = codegenoptions::DebugLineTablesOnly;
+
   RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind, DWARFVersion,
   DebuggerTuning);
 


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -64,6 +64,12 @@
 // RUN: %clang -### -c -g %s -target arm64-apple-tvos9.0 2>&1 \
 // RUN: | FileCheck -check-prefix=G_STANDALONE \
 // RUN: -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=GLTO_ONLY %s
+// RUN: %clang -### -c -g -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=G_STANDALONE %s
 
 // FreeBSD.
 // RUN: %clang -### -c -g %s -target x86_64-pc-freebsd11.0 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3717,6 +3717,11 @@
   // Adjust the debug info kind for the given toolchain.
   TC.adjustDebugInfoKind(DebugInfoKind, Args);
 
+  // When emitting remarks, we need at least debug lines in the output.
+  if (shouldEmitRemarks(Args) &&
+  DebugInfoKind <= codegenoptions::DebugDirectivesOnly)
+DebugInfoKind = codegenoptions::DebugLineTablesOnly;
+
   RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind, DWARFVersion,
   DebuggerTuning);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71325: [Remarks][Driver] Ask for line tables when remarks are enabled

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg updated this revision to Diff 233459.

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

https://reviews.llvm.org/D71325

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -64,6 +64,12 @@
 // RUN: %clang -### -c -g %s -target arm64-apple-tvos9.0 2>&1 \
 // RUN: | FileCheck -check-prefix=G_STANDALONE \
 // RUN: -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=GLTO_ONLY %s
+// RUN: %clang -### -c -g -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=G_STANDALONE %s
 
 // FreeBSD.
 // RUN: %clang -### -c -g %s -target x86_64-pc-freebsd11.0 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3717,6 +3717,11 @@
   // Adjust the debug info kind for the given toolchain.
   TC.adjustDebugInfoKind(DebugInfoKind, Args);
 
+  // When emitting remarks, we need at least debug lines in the output.
+  if (shouldEmitRemarks(Args) &&
+  DebugInfoKind <= codegenoptions::DebugDirectivesOnly)
+DebugInfoKind = codegenoptions::DebugLineTablesOnly;
+
   RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind, DWARFVersion,
   DebuggerTuning);
 


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -64,6 +64,12 @@
 // RUN: %clang -### -c -g %s -target arm64-apple-tvos9.0 2>&1 \
 // RUN: | FileCheck -check-prefix=G_STANDALONE \
 // RUN: -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=GLTO_ONLY %s
+// RUN: %clang -### -c -g -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=G_STANDALONE %s
 
 // FreeBSD.
 // RUN: %clang -### -c -g %s -target x86_64-pc-freebsd11.0 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3717,6 +3717,11 @@
   // Adjust the debug info kind for the given toolchain.
   TC.adjustDebugInfoKind(DebugInfoKind, Args);
 
+  // When emitting remarks, we need at least debug lines in the output.
+  if (shouldEmitRemarks(Args) &&
+  DebugInfoKind <= codegenoptions::DebugDirectivesOnly)
+DebugInfoKind = codegenoptions::DebugLineTablesOnly;
+
   RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind, DWARFVersion,
   DebuggerTuning);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71325: [Remarks][Driver] Ask for line tables when remarks are enabled

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg created this revision.
thegameg added reviewers: JDevlieghere, friss, aprantl.
Herald added a project: clang.

Serialized remarks contain debug locations for each remark, by storing a file 
path, a line, and a column.

Also, remarks support being embedded in a .dSYM bundle using a separate section 
in object files, that is found by `dsymutil` through the debug map.

In order for tools to map addresses to source and display remarks in the 
source, we need line tables, and in order for `dsymutil` to find the object 
files containing the remark section, we need to keep the debug map around.


Repository:
  rC Clang

https://reviews.llvm.org/D71325

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -64,6 +64,12 @@
 // RUN: %clang -### -c -g %s -target arm64-apple-tvos9.0 2>&1 \
 // RUN: | FileCheck -check-prefix=G_STANDALONE \
 // RUN: -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=GLTO_ONLY %s
+// RUN: %clang -### -c -g -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=G_STANDALONE %s
 
 // FreeBSD.
 // RUN: %clang -### -c -g %s -target x86_64-pc-freebsd11.0 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3717,6 +3717,13 @@
   // Adjust the debug info kind for the given toolchain.
   TC.adjustDebugInfoKind(DebugInfoKind, Args);
 
+  // When emitting remarks, we need at least debug lines in the output.
+  if (shouldEmitRemarks(Args) &&
+  (DebugInfoKind == codegenoptions::NoDebugInfo ||
+   DebugInfoKind == codegenoptions::LocTrackingOnly ||
+   DebugInfoKind == codegenoptions::DebugDirectivesOnly))
+DebugInfoKind = codegenoptions::DebugLineTablesOnly;
+
   RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind, DWARFVersion,
   DebuggerTuning);
 


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -64,6 +64,12 @@
 // RUN: %clang -### -c -g %s -target arm64-apple-tvos9.0 2>&1 \
 // RUN: | FileCheck -check-prefix=G_STANDALONE \
 // RUN: -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=GLTO_ONLY %s
+// RUN: %clang -### -c -g -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=G_STANDALONE %s
 
 // FreeBSD.
 // RUN: %clang -### -c -g %s -target x86_64-pc-freebsd11.0 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3717,6 +3717,13 @@
   // Adjust the debug info kind for the given toolchain.
   TC.adjustDebugInfoKind(DebugInfoKind, Args);
 
+  // When emitting remarks, we need at least debug lines in the output.
+  if (shouldEmitRemarks(Args) &&
+  (DebugInfoKind == codegenoptions::NoDebugInfo ||
+   DebugInfoKind == codegenoptions::LocTrackingOnly ||
+   DebugInfoKind == codegenoptions::DebugDirectivesOnly))
+DebugInfoKind = codegenoptions::DebugLineTablesOnly;
+
   RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind, DWARFVersion,
   DebuggerTuning);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70748: [clang test] Do not assume default target

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

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

LGTM, thanks for fixing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70748



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

[PATCH] D68611: [IRGen] Emit lifetime markers for temporary struct allocas

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
thegameg marked an inline comment as done.
Closed by commit rG143f6b837790: [IRGen] Emit lifetime markers for temporary 
struct allocas (authored by thegameg).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68611

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/aarch64-byval-temp.c

Index: clang/test/CodeGen/aarch64-byval-temp.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-byval-temp.c
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -emit-llvm -triple arm64-- -o - %s -O0 | FileCheck %s --check-prefix=CHECK-O0
+// RUN: %clang_cc1 -emit-llvm -disable-llvm-optzns -triple arm64-- -o - %s -O3 | FileCheck %s --check-prefix=CHECK-O3
+
+struct large {
+void* pointers[8];
+};
+
+void pass_large(struct large);
+
+// For arm64, we don't use byval to pass structs but instead we create
+// temporary allocas.
+//
+// Make sure we generate the appropriate lifetime markers for the temporary
+// allocas so that the optimizer can re-use stack slots if possible.
+void example() {
+struct large l = {0};
+pass_large(l);
+pass_large(l);
+}
+// CHECK-O0-LABEL: define void @example(
+// The alloca for the struct on the stack.
+// CHECK-O0: %[[l:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+// The alloca for the temporary stack space that we use to pass the argument.
+// CHECK-O0-NEXT: %[[byvaltemp:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+// Another one to pass the argument to the second function call.
+// CHECK-O0-NEXT: %[[byvaltemp1:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+// First, memset `l` to 0.
+// CHECK-O0-NEXT: %[[bitcastl:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[l]] to i8*
+// CHECK-O0-NEXT: call void @llvm.memset.p0i8.i64(i8* align 8 %[[bitcastl]], i8 0, i64 64, i1 false)
+// Then, memcpy `l` to the temporary stack space.
+// CHECK-O0-NEXT: %[[src:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[byvaltemp]] to i8*
+// CHECK-O0-NEXT: %[[dst:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[l]] to i8*
+// CHECK-O0-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %[[src]], i8* align 8 %[[dst]], i64 64, i1 false)
+// Finally, call using a pointer to the temporary stack space.
+// CHECK-O0-NEXT: call void @pass_large(%struct.large* %[[byvaltemp]])
+// Now, do the same for the second call, using the second temporary alloca.
+// CHECK-O0-NEXT: %[[src:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[byvaltemp1]] to i8*
+// CHECK-O0-NEXT: %[[dst:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[l]] to i8*
+// CHECK-O0-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %[[src]], i8* align 8 %[[dst]], i64 64, i1 false)
+// CHECK-O0-NEXT: call void @pass_large(%struct.large* %[[byvaltemp1]])
+// CHECK-O0-NEXT: ret void
+//
+// At O3, we should have lifetime markers to help the optimizer re-use the temporary allocas.
+//
+// CHECK-O3-LABEL: define void @example(
+// The alloca for the struct on the stack.
+// CHECK-O3: %[[l:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+// The alloca for the temporary stack space that we use to pass the argument.
+// CHECK-O3-NEXT: %[[byvaltemp:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+// Another one to pass the argument to the second function call.
+// CHECK-O3-NEXT: %[[byvaltemp1:[0-9A-Za-z-]+]] = alloca %struct.large, align 8
+//
+// Mark the start of the lifetime for `l`
+// CHECK-O3-NEXT: %[[bitcastl:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[l]] to i8*
+// CHECK-O3-NEXT: call void @llvm.lifetime.start.p0i8(i64 64, i8* %[[bitcastl]])
+//
+// First, memset `l` to 0.
+// CHECK-O3-NEXT: %[[bitcastl:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[l]] to i8*
+// CHECK-O3-NEXT: call void @llvm.memset.p0i8.i64(i8* align 8 %[[bitcastl]], i8 0, i64 64, i1 false)
+//
+// Lifetime of the first temporary starts here and ends right after the call.
+// CHECK-O3-NEXT: %[[bitcastbyvaltemp:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[byvaltemp]] to i8*
+// CHECK-O3-NEXT: call void @llvm.lifetime.start.p0i8(i64 64, i8* %[[bitcastbyvaltemp]])
+//
+// Then, memcpy `l` to the temporary stack space.
+// CHECK-O3-NEXT: %[[src:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[byvaltemp]] to i8*
+// CHECK-O3-NEXT: %[[dst:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[l]] to i8*
+// CHECK-O3-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 %[[src]], i8* align 8 %[[dst]], i64 64, i1 false)
+// Finally, call using a pointer to the temporary stack space.
+// CHECK-O3-NEXT: call void @pass_large(%struct.large* %[[byvaltemp]])
+//
+// The lifetime of the temporary used to pass a pointer to the struct ends here.
+// CHECK-O3-NEXT: %[[bitcastbyvaltemp:[0-9A-Za-z-]+]] = bitcast %struct.large* %[[byvaltemp]] to i8*
+// CHECK-O3-NEXT: call void @llvm.lifetime.end.p0i8(i64 64, i8* %[[bitcastbyvaltemp]])
+//

[PATCH] D67683: [Timers] Fix printing some `-ftime-report` sections twice. Fixes PR40328.

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

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

LGTM, thanks!


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

https://reviews.llvm.org/D67683



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

[PATCH] D65000: [ARM] Set default alignment to 64bits

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

In D65000#1616368 , @dnsampaio wrote:

> I have tested this in our MacOS and linux environments. @thakis @thegameg 
> @phosek, would it be possible for you to check if this works for you?


I just built trunk with this patch applied and `$ ninja check-clang-codegen` 
seems to pass. Thanks @dnsampaio!


Repository:
  rC Clang

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

https://reviews.llvm.org/D65000



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

[PATCH] D65000: [ARM] Set default alignment to 64bits

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added inline comments.



Comment at: cfe/trunk/test/CodeGen/ARM/exception-alignment.cpp:9
+// A16-NEXT: store <2 x i64> , <2 x i64>* [[BC]], align 16
+#include 
+

thakis wrote:
> This fails on some bots:
> 
> http://lab.llvm.org:8011/builders/clang-atom-d525-fedora-rel/builds/26891/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Aexception-alignment.cpp
> 
> ```
> In file included from 
> /export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/tools/clang/test/CodeGen/ARM/exception-alignment.cpp:9:
> In file included from 
> /export/users/atombot/llvm/clang-atom-d525-fedora-rel/stage1/lib/clang/10.0.0/include/arm_neon.h:31:
> In file included from 
> /export/users/atombot/llvm/clang-atom-d525-fedora-rel/stage1/lib/clang/10.0.0/include/stdint.h:52:
> In file included from /usr/include/stdint.h:26:
> In file included from /usr/include/bits/libc-header-start.h:33:
> In file included from /usr/include/features.h:452:
> /usr/include/gnu/stubs.h:7:11: fatal error: 'gnu/stubs-32.h' file not found
> # include 
>   ^~~~
> 1 error generated.
> FileCheck error: '-' is empty.
> FileCheck command line:  
> /export/users/atombot/llvm/clang-atom-d525-fedora-rel/stage1/bin/FileCheck 
> --check-prefixes=CHECK,A16 
> /export/users/atombot/llvm/clang-atom-d525-fedora-rel/llvm/tools/clang/test/CodeGen/ARM/exception-alignment.cpp
> 
> --
> ```
It also fails on Darwin:

http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/245/consoleFull

```
Command Output (stderr):
--
In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/clang/test/CodeGen/ARM/exception-alignment.cpp:9:
In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/10.0.0/include/arm_neon.h:31:
In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/10.0.0/include/stdint.h:52:
In file included from /usr/include/stdint.h:52:
In file included from /usr/include/sys/_types.h:32:
/usr/include/sys/cdefs.h:763:2: error: Unsupported architecture
#error Unsupported architecture
 ^
In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/clang/test/CodeGen/ARM/exception-alignment.cpp:9:
In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/10.0.0/include/arm_neon.h:31:
In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/clang-build/lib/clang/10.0.0/include/stdint.h:52:
In file included from /usr/include/stdint.h:52:
In file included from /usr/include/sys/_types.h:33:
/usr/include/machine/_types.h:34:2: error: architecture not supported
#error architecture not supported
 ^
```


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65000



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

[PATCH] D63518: BitStream reader: propagate errors

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added inline comments.



Comment at: llvm/include/llvm/Bitcode/BitstreamReader.h:489
 
   bool ReadBlockEnd() {
 if (BlockScope.empty()) return true;

jfb wrote:
> thegameg wrote:
> > Any reason why this doesn't return `Error`?
> I'm not sure it's really an error: it goes to the end of the block either 
> because it's empty, or because it pops the scope (which doesn't error 
> either). It might be erroneously used, but I'm not sure we should make it an 
> error right now. WDYT?
Yeah, I'm not sure either... `BitstreamCursor::advance` seems to return 
`BitstreamEntry::getError();` if this function fails after it encountered an 
`END_BLOCK`, and the other users seem to return things like 
`SDError::InvalidDiagnostics` or `Cursor::BadBlock`.

I guess for now, it's fine as it is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63518



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

[PATCH] D63518: BitStream reader: propagate errors

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added inline comments.



Comment at: llvm/include/llvm/Bitcode/BitstreamReader.h:441
   // If we found a sub-block, just skip over it and check the next entry.
-  if (SkipBlock())
-return BitstreamEntry::getError();
+  if (llvm::Error Err = SkipBlock())
+return std::move(Err);

`llvm::` seems unnecessary here.



Comment at: llvm/include/llvm/Bitcode/BitstreamReader.h:489
 
   bool ReadBlockEnd() {
 if (BlockScope.empty()) return true;

Any reason why this doesn't return `Error`?



Comment at: llvm/lib/Bitcode/Reader/BitstreamReader.cpp:140
 CodeOp.getEncoding() == BitCodeAbbrevOp::Blob)
   report_fatal_error("Abbreviation starts with an Array or a Blob");
+Expected MaybeCode = readAbbreviatedField(*this, CodeOp);

`return createStringError` here too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63518



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

[PATCH] D62709: Fix -DBUILD_SHARED_LIBS=ON build after rL362160

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

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

Sorry about this! This LGTM, thanks for fixing it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62709



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

[PATCH] D58996: [Remarks] Refactor remark diagnostic emission in a RemarkStreamer

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC355507: [Remarks] Refactor remark diagnostic emission in a 
RemarkStreamer (authored by thegameg, committed by ).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58996?vs=189425=189500#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58996

Files:
  lib/CodeGen/CodeGenAction.cpp


Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -30,6 +30,7 @@
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/RemarkStreamer.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/Linker/Linker.h"
 #include "llvm/Pass.h"
@@ -276,8 +277,8 @@
   return;
 }
 
-Ctx.setDiagnosticsOutputFile(
-llvm::make_unique(OptRecordFile->os()));
+Ctx.setRemarkStreamer(llvm::make_unique(
+CodeGenOpts.OptRecordFile, OptRecordFile->os()));
 
 if (CodeGenOpts.getProfileUse() != CodeGenOptions::ProfileNone)
   Ctx.setDiagnosticsHotnessRequested(true);


Index: lib/CodeGen/CodeGenAction.cpp
===
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -30,6 +30,7 @@
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/RemarkStreamer.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/Linker/Linker.h"
 #include "llvm/Pass.h"
@@ -276,8 +277,8 @@
   return;
 }
 
-Ctx.setDiagnosticsOutputFile(
-llvm::make_unique(OptRecordFile->os()));
+Ctx.setRemarkStreamer(llvm::make_unique(
+CodeGenOpts.OptRecordFile, OptRecordFile->os()));
 
 if (CodeGenOpts.getProfileUse() != CodeGenOptions::ProfileNone)
   Ctx.setDiagnosticsHotnessRequested(true);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55775: [Driver] Don't override '-march' when using '-arch x86_64h'

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

In D55775#1338512 , @qcolombet wrote:

> Should we emit an error if we request x86_64h with an arch older than haswell?


Makes sense. I'll put up a patch soon.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55775



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

[PATCH] D55775: [Driver] Don't override '-march' when using '-arch x86_64h'

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL349381: [Driver] Dont override -march when 
using -arch x86_64h (authored by thegameg, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55775?vs=178486=178504#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55775

Files:
  cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
  cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
  cfe/trunk/test/Driver/clang-translation.c


Index: cfe/trunk/test/Driver/clang-translation.c
===
--- cfe/trunk/test/Driver/clang-translation.c
+++ cfe/trunk/test/Driver/clang-translation.c
@@ -33,6 +33,11 @@
 // AVX2: "-target-cpu"
 // AVX2: "core-avx2"
 
+// RUN: %clang -target x86_64h-apple-darwin -march=skx -### %s -o /dev/null 
2>&1 | \
+// RUN: FileCheck -check-prefix=X8664HSKX %s
+// X8664HSKX: "-target-cpu"
+// X8664HSKX: "skx"
+
 // RUN: %clang -target i386-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
 // RUN: FileCheck -check-prefix=PENRYN %s
 // RUN: %clang -target x86_64-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
Index: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
@@ -2017,12 +2017,8 @@
 else if (Name == "pentIIm3")
   DAL->AddJoinedArg(nullptr, MArch, "pentium2");
 
-else if (Name == "x86_64")
+else if (Name == "x86_64" || Name == "x86_64h")
   DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-else if (Name == "x86_64h") {
-  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-  DAL->AddJoinedArg(nullptr, MArch, "x86_64h");
-}
 
 else if (Name == "arm")
   DAL->AddJoinedArg(nullptr, MArch, "armv4t");
Index: cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
@@ -23,12 +23,8 @@
 const char *x86::getX86TargetCPU(const ArgList ,
  const llvm::Triple ) {
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
-if (StringRef(A->getValue()) != "native") {
-  if (Triple.isOSDarwin() && Triple.getArchName() == "x86_64h")
-return "core-avx2";
-
+if (StringRef(A->getValue()) != "native")
   return A->getValue();
-}
 
 // FIXME: Reject attempts to use -march=native unless the target matches
 // the host.


Index: cfe/trunk/test/Driver/clang-translation.c
===
--- cfe/trunk/test/Driver/clang-translation.c
+++ cfe/trunk/test/Driver/clang-translation.c
@@ -33,6 +33,11 @@
 // AVX2: "-target-cpu"
 // AVX2: "core-avx2"
 
+// RUN: %clang -target x86_64h-apple-darwin -march=skx -### %s -o /dev/null 2>&1 | \
+// RUN: FileCheck -check-prefix=X8664HSKX %s
+// X8664HSKX: "-target-cpu"
+// X8664HSKX: "skx"
+
 // RUN: %clang -target i386-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
 // RUN: FileCheck -check-prefix=PENRYN %s
 // RUN: %clang -target x86_64-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
Index: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
@@ -2017,12 +2017,8 @@
 else if (Name == "pentIIm3")
   DAL->AddJoinedArg(nullptr, MArch, "pentium2");
 
-else if (Name == "x86_64")
+else if (Name == "x86_64" || Name == "x86_64h")
   DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-else if (Name == "x86_64h") {
-  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-  DAL->AddJoinedArg(nullptr, MArch, "x86_64h");
-}
 
 else if (Name == "arm")
   DAL->AddJoinedArg(nullptr, MArch, "armv4t");
Index: cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/X86.cpp
@@ -23,12 +23,8 @@
 const char *x86::getX86TargetCPU(const ArgList ,
  const llvm::Triple ) {
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
-if (StringRef(A->getValue()) != "native") {
-  if (Triple.isOSDarwin() && Triple.getArchName() == "x86_64h")
-return "core-avx2";
-
+if (StringRef(A->getValue()) != "native")
   return A->getValue();
-}
 
 // FIXME: Reject attempts to use -march=native unless the target matches
 // the host.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D55775: [Driver] Don't override '-march' when using '-arch x86_64h'

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg created this revision.
thegameg added reviewers: vsk, dexonsmith, ab, t.p.northover, Gerolf.

On Darwin, using '-arch x86_64h' would always override the option passed 
through '-march'.

This patch allows users to use '-march' with x86_64h, while keeping the default 
to 'core-avx2'


https://reviews.llvm.org/D55775

Files:
  lib/Driver/ToolChains/Arch/X86.cpp
  lib/Driver/ToolChains/Darwin.cpp
  test/Driver/clang-translation.c


Index: test/Driver/clang-translation.c
===
--- test/Driver/clang-translation.c
+++ test/Driver/clang-translation.c
@@ -33,6 +33,11 @@
 // AVX2: "-target-cpu"
 // AVX2: "core-avx2"
 
+// RUN: %clang -target x86_64h-apple-darwin -march=skx -### %s -o /dev/null 
2>&1 | \
+// RUN: FileCheck -check-prefix=X8664HSKX %s
+// X8664HSKX: "-target-cpu"
+// X8664HSKX: "skx"
+
 // RUN: %clang -target i386-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
 // RUN: FileCheck -check-prefix=PENRYN %s
 // RUN: %clang -target x86_64-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1967,12 +1967,8 @@
 else if (Name == "pentIIm3")
   DAL->AddJoinedArg(nullptr, MArch, "pentium2");
 
-else if (Name == "x86_64")
+else if (Name == "x86_64" || Name == "x86_64h")
   DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-else if (Name == "x86_64h") {
-  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-  DAL->AddJoinedArg(nullptr, MArch, "x86_64h");
-}
 
 else if (Name == "arm")
   DAL->AddJoinedArg(nullptr, MArch, "armv4t");
Index: lib/Driver/ToolChains/Arch/X86.cpp
===
--- lib/Driver/ToolChains/Arch/X86.cpp
+++ lib/Driver/ToolChains/Arch/X86.cpp
@@ -23,12 +23,8 @@
 const char *x86::getX86TargetCPU(const ArgList ,
  const llvm::Triple ) {
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
-if (StringRef(A->getValue()) != "native") {
-  if (Triple.isOSDarwin() && Triple.getArchName() == "x86_64h")
-return "core-avx2";
-
+if (StringRef(A->getValue()) != "native")
   return A->getValue();
-}
 
 // FIXME: Reject attempts to use -march=native unless the target matches
 // the host.


Index: test/Driver/clang-translation.c
===
--- test/Driver/clang-translation.c
+++ test/Driver/clang-translation.c
@@ -33,6 +33,11 @@
 // AVX2: "-target-cpu"
 // AVX2: "core-avx2"
 
+// RUN: %clang -target x86_64h-apple-darwin -march=skx -### %s -o /dev/null 2>&1 | \
+// RUN: FileCheck -check-prefix=X8664HSKX %s
+// X8664HSKX: "-target-cpu"
+// X8664HSKX: "skx"
+
 // RUN: %clang -target i386-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
 // RUN: FileCheck -check-prefix=PENRYN %s
 // RUN: %clang -target x86_64-apple-macosx10.12 -### -S %s -o %t.s 2>&1 | \
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1967,12 +1967,8 @@
 else if (Name == "pentIIm3")
   DAL->AddJoinedArg(nullptr, MArch, "pentium2");
 
-else if (Name == "x86_64")
+else if (Name == "x86_64" || Name == "x86_64h")
   DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-else if (Name == "x86_64h") {
-  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_m64));
-  DAL->AddJoinedArg(nullptr, MArch, "x86_64h");
-}
 
 else if (Name == "arm")
   DAL->AddJoinedArg(nullptr, MArch, "armv4t");
Index: lib/Driver/ToolChains/Arch/X86.cpp
===
--- lib/Driver/ToolChains/Arch/X86.cpp
+++ lib/Driver/ToolChains/Arch/X86.cpp
@@ -23,12 +23,8 @@
 const char *x86::getX86TargetCPU(const ArgList ,
  const llvm::Triple ) {
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
-if (StringRef(A->getValue()) != "native") {
-  if (Triple.isOSDarwin() && Triple.getArchName() == "x86_64h")
-return "core-avx2";
-
+if (StringRef(A->getValue()) != "native")
   return A->getValue();
-}
 
 // FIXME: Reject attempts to use -march=native unless the target matches
 // the host.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44093: [BUILTINS] structure pretty printer

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

Thanks for working on this! Few remarks in the comments.




Comment at: lib/CodeGen/CGBuiltin.cpp:934
 
+static Value *dumpRecord(CodeGenFunction , QualType RType,
+ Value*& RecordPtr, CharUnits Align,

`llvm::Value` for consistency?



Comment at: lib/CodeGen/CGBuiltin.cpp:966
+Types[Context.getPointerType(Context.CharTy)] = "%s";
+}
+

Indentation failed here.



Comment at: lib/CodeGen/CGBuiltin.cpp:976
+  FieldPtr = CGF.Builder.CreateAdd(FieldPtr, 
ConstantInt::get(CGF.IntPtrTy, Off));
+  FieldPtr = CGF.Builder.CreateIntToPtr(FieldPtr, CGF.VoidPtrTy);
+}

I think you should use `getelementptr` instead of `ptrtoint` -> `inttoptr` 
https://llvm.org/docs/GetElementPtr.html



Comment at: lib/CodeGen/CGBuiltin.cpp:997
+if (Types.find(CanonicalType) == Types.end())
+Format = Types[Context.VoidPtrTy];
+else

Indentation failed here too.



Comment at: lib/CodeGen/CGBuiltin.cpp:1003
+llvm::Type *ResType = CGF.ConvertType(ResPtrType);
+FieldPtr = CGF.Builder.CreatePointerCast(FieldPtr, ResType);
+Address FieldAddress = Address(FieldPtr, Align);

If you use GEP you should be able to get rid of this cast here.



Comment at: lib/CodeGen/CGBuiltin.cpp:1009
+
+GString = CGF.Builder.CreateGlobalStringPtr(Format + "\n");
+TmpRes = CGF.Builder.CreateCall(Func, {GString, FieldPtr});

You can probably use `llvm::Twine` for the concatenation of `Format`: 
http://llvm.org/doxygen/classllvm_1_1Twine.html.



Comment at: lib/Sema/SemaChecking.cpp:1159
+const FunctionProtoType *FT = dyn_cast(FuncType);
+if (FT) {
+  if (!FT->isVariadic() ||

`if (const FunctionProtoType *FT = dyn_cast(FuncType))`


Repository:
  rC Clang

https://reviews.llvm.org/D44093



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

[PATCH] D26953: clang-format: handle formatting on constexpr if

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

In https://reviews.llvm.org/D26953#783497, @jtbandes wrote:

> Hm, I probably should've searched first — but I just re-implemented this in 
> https://reviews.llvm.org/D34330.  Actually, I think my implementation solves 
> the `AllowShortIfStatementsOnASingleLine` issue you were mentioning here 


No problem! Great!


https://reviews.llvm.org/D26953



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

[PATCH] D26953: clang-format: handle formatting on constexpr if

[Francis Visoiu Mistrih via Phabricator via cfe-commits]

thegameg added a comment.

In https://reviews.llvm.org/D26953#751021, @thakis wrote:

> This looks good to me, thanks. Sorry about the slow turnaround. Do you have 
> commit access? If not, I can land it for you – but it also looks like you've 
> contributed several patches by now, so you could also ask for commit access 
> if you don't have it yet.


Thank you for the review. As @gnzlbg pointed out here (PR28867) 
, I tested with 
`AllowShortIfStatementsOnASingleLine` and it does not seem to work as intended. 
I didn't have time to fix that yet, so I'll probably delay this for now. If 
anyone has time, feel free to fix + land this instead of me.


https://reviews.llvm.org/D26953



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