[PATCH] D50099: [DebugInfo][OpenCL] Address post-commit review of D49930

2018-08-06 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D50099#1189667, @scott.linder wrote:

> When I went to mark these as static I noticed they use 
> `CGDebugInfo::CreateMemberType` which uses a couple other non-static member 
> functions, and it starts to become difficult to tease things out into nice 
> clean static functions.


Ah. Fair. No worries then.




Comment at: lib/CodeGen/CGDebugInfo.cpp:997
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagAppleBlock;
   unsigned LineNo = 0;
 

scott.linder wrote:
> echristo wrote:
> > Just noticed that LineNo is 0... for the entire function.
> What should it be instead? My understanding is that LineNo=0 indicates there 
> is no corresponding source, and these fields seem to be implementation 
> details.
Could probably just replace it with a constant 0 in the calls rather than 
having the local?


https://reviews.llvm.org/D50099



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


[PATCH] D50099: [DebugInfo][OpenCL] Address post-commit review of D49930

2018-08-07 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


https://reviews.llvm.org/D50099



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


[PATCH] D51150: [x86/retpoline] Split the LLVM concept of retpolines into separate subtarget features for indirect calls and indirect branches.

2018-08-22 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


Repository:
  rL LLVM

https://reviews.llvm.org/D51150



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


[PATCH] D47070: [CUDA] Upgrade linked bitcode to enable inlining

2018-05-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D47070#1105533, @Hahnfeld wrote:

> Looks like this was added as a "temporary solution" in 
> https://reviews.llvm.org/D8984. Meanwhile the attribute whitelist was merged 
> half a year later (https://reviews.llvm.org/D7802), so maybe we can just get 
> rid of comparing `target-cpu` and `target-features` entirely?


You don't want to get rid of the comparison because you might have specific 
code that can't be inlined from one into another.

In https://reviews.llvm.org/D47070#1104846, @tra wrote:

> This was not intended. :-( I was unaware that GetCPUAndFeaturesAttributes() 
> would add any feature that looks like a valid CPU name to the target-cpu 
> attribute.
>  All I needed is to make builtins available or not. Setting them as function 
> attributes is not what we need here.
>
> I'm not sure what's the best way to deal with this. On one hand I do need to 
> make some builtins available depending on combination of GPU arch and PTX 
> version. The only way to do it is via the features. On the other hand, the 
> features appear to propagate to LLVM IR, which is something I don't need or 
> want.
>
> One way would be to introduce some sort of feature blacklist which would 
> prevent them from being converted to function attributes.
>  Or, perhaps, we can change TARGET_BUILTIN or create something similar which 
> would allow availability of builtins w/o relying on features.
>
> As a short-term fix we can disable feature-to-function attribute propagation 
> for NVPTX until we fix it.
>
> @echristo -- any other suggestions?


This is some of what I was talking about when I was mentioning how function 
attributes and the targets work. Ideally you'll have a compatible set of 
features and it won't really cause an issue. The idea is that if you're 
compiling for a minimum ptx feature of X, then any "compatible" set of ptx 
should be able to inline into your code. I think you do want the features to 
propagate in general, just specific use cases may not care one way or another - 
that said, for those use cases you're probably just compiling everything with 
the same feature anyhow.

I guess, ultimately, I'm not seeing what the concern here is for how features 
are working or not working for the target so it's harder to help. What is the 
problem you're running into, or can you try a different way of explaining it to 
me? :)


Repository:
  rC Clang

https://reviews.llvm.org/D47070



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


[PATCH] D45783: [DEBUGINFO, NVPTX] Render `-no-cuda-debug` LLVM option when required.

2018-05-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

So, I'd really prefer not to set options via the backend option path. From here 
I think we should aim to take all of the options we added and having the asm 
printer in the backend know how to set them depending on the target. We could 
also add things to the IR metadata if necessary, but I'd like to avoid that if 
possible.

Thoughts?


Repository:
  rC Clang

https://reviews.llvm.org/D45783



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


[PATCH] D47029: [X86] Remove some preprocessor feature checks from intrinsic headers

2018-05-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM.

-eric


https://reviews.llvm.org/D47029



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


[PATCH] D46791: Make -gsplit-dwarf generally available

2018-05-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo added subscribers: pcc, paulsemel.
echristo added a comment.

FWIW Peter has some patches to move object emission away from objcopy that I'm 
on the hook to review here shortly so the objcopy part of this should become 
unnecessary and can just have us able to emit dwarf5 compatible split dwarf on 
any platform.

-eric


https://reviews.llvm.org/D46791



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


[PATCH] D47050: MC: Change the streamer ctors to take an object writer instead of a stream. NFCI.

2018-05-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D47050



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


[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

2018-05-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

Well, that's ridiculous. We should really fix this a better way in the future.

That said, would you add a testcase for this please so we don't regress if we 
fix it? :)

-eric


Repository:
  rC Clang

https://reviews.llvm.org/D46230



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


[PATCH] D46541: [CodeGen] Improve diagnostics related to target attributes

2018-05-21 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

I think this will work, one inline comment. Might also be good to get a few 
different test cases, e.g. one where we're not seeing the alphabetically first 
as the minimum :)




Comment at: lib/CodeGen/CodeGenModule.h:1085
 
+  TargetAttr::ParsedTargetAttr filterFunctionTargetAttrs(const TargetAttr *TD);
+

Needs a block comment saying what it does :)


https://reviews.llvm.org/D46541



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


[PATCH] D46791: Make -gsplit-dwarf generally available

2018-05-21 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D46791#1107168, @pcc wrote:

> There were a bunch of them but the last one was 
> https://reviews.llvm.org/D47093 which has already landed :)
>
> Probably all that needs to happen on this change is to replace the isLinux() 
> check with isELF().


Be good if it didn't even require the isELF check anymore, but that probably 
requires testing on a few platforms. :)


https://reviews.llvm.org/D46791



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


[PATCH] D38596: Implement attribute target multiversioning

2017-10-05 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

This generally works for me modulo the things that Hal has mentioned. You'll 
probably want to add Richard to the review list for the Sema bits as well.

Thanks!




Comment at: lib/Sema/SemaDecl.cpp:3214
   if (!isFriend && NewMethod->getLexicalDeclContext()->isRecord() &&
-  !IsClassScopeExplicitSpecialization) {
+  !IsClassScopeExplicitSpecialization ) {
 //-- Member function declarations with the same name and the

Extra whitespace.


https://reviews.llvm.org/D38596



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


[PATCH] D38903: [ubsan] Only use indirect RTTI in prologues on Darwin

2017-10-13 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

Given you were the last one in this code it seems reasonable to let you go for 
it :)

That said, I didn't notice anything in particular that stuck out at me.


https://reviews.llvm.org/D38903



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


[PATCH] D39179: [AArch64] Fix PR34625 -mtune without -mcpu should not set -target-cpu

2017-10-23 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

This should be fine for now. Thanks!

-eric


https://reviews.llvm.org/D39179



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


[PATCH] D51177: [DEBUGINFO] Add support for emission of the debug directives only.

2018-08-24 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

Should we just have them mean the same thing and change it based on target?


Repository:
  rC Clang

https://reviews.llvm.org/D51177



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


[PATCH] D51007: Test the cross-product of options that affect how libgcc-related arguments are passed to the linker.

2018-08-28 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

Feel free to add any tests like this that test existing behavior without review 
in the future. If we want to change the behavior we should probably review that 
though :)

-eric


Repository:
  rC Clang

https://reviews.llvm.org/D51007



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


[PATCH] D51521: Refactor Addlibgcc to make the when and what logic more straightfoward.

2018-08-30 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

LGTM, but let's get Stephen to ack as well.


Repository:
  rC Clang

https://reviews.llvm.org/D51521



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


[PATCH] D51554: [CUDA][OPENMP][NVPTX]Improve logic of the debug info support.

2018-09-04 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a reviewer: dblaikie.
echristo added a comment.

The change in name here from "line tables" to "directives only" feels a bit 
confusing.  "Limited" seems to be a bit more clear, or even remaining line 
tables only. Can you explain where you were going with this particular set of 
changes in a bit more detail please?

Thanks!

-eric


Repository:
  rC Clang

https://reviews.llvm.org/D51554



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


[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

2018-07-19 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

Getting close, one inline comment.




Comment at: lib/Driver/ToolChains/Cuda.h:161
   bool SupportsProfiling() const override { return false; }
+  bool supportsDebugInfoOption(const llvm::opt::Arg *) const override {
+return false;

I'd like to see this for all of the possible options (returning false by 
default is fine), with the ones supported spelled out (-gdwarf/-gdwarf-2/etc). 
This means you'll probably have to check this at all option processing sites 
for debug info. Alternately you could check g_group flags as a whole first and 
then process individually.


Repository:
  rC Clang

https://reviews.llvm.org/D49148



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


[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

2018-07-25 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:933-938
+  if (TC.supportsDebugInfoOption(A)) {
+Action();
+return true;
+  }
+  reportUnsupportedDebugInfoOption(A, Args, D, TC.getTriple());
+  return false;

I'd probably simplify this as:

```
if (TC.supportsDebugInfoOption(A))
  return true;
reportUnsupportedDebugInfoOption(A, Args, D, TC.getTriple());
return false;
```

And just leave the action part in the rest of the code.  I think that means you 
could also leave the bool off.

As another optimization here you could, instead, just check all of the 
arguments ahead of time by getting the full list of debug info and just 
checking them all. That's probably not worth it though TBH.
   


Repository:
  rC Clang

https://reviews.llvm.org/D49148



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


[PATCH] D49828: [libc++] Add hack to allow ubsan to work w/o compiler-rt (__muloti4 is undefined)

2018-07-25 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

Bit of a hack, but I'm ok with it.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49828



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


[PATCH] D49930: [DebugInfo][OpenCL] Generate correct block literal debug info for OpenCL

2018-07-30 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

The patch is fine, in general, couple of comments:

a) Can you refactor this so the if conditionals are just two functions? Those 
functions are big enough already.
b) I'm not quite sure why you're picking limited here, do you have an 
explanation?
c) Can you split that part out into a separate test? Additional run lines in 
the existing blocks.cl test would be fine.

Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D49930



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


[PATCH] D49930: [DebugInfo][OpenCL] Generate correct block literal debug info for OpenCL

2018-07-30 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D49930#1181000, @scott.linder wrote:

> Sorry, I didn't see the additional comments until after I committed. I will 
> make those changes; is it OK to update this review, or should I create a new 
> one?


A new one is great. Just treat this as post commit :)

> As for choosing limited it was just what the function adding the debug info 
> checked for as a minimum; what would you generally choose instead?

I'd have just suggested plain -g for it?


Repository:
  rL LLVM

https://reviews.llvm.org/D49930



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


[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

2018-08-01 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D49148#1184957, @tra wrote:

> I wonder, what's the right thing to do to silence the warnings. For instance, 
> we compile everything with -Werror and the warnings result in build breaks.
>
> Easy way out is to pass `-Wno-unsupported-target-opt`.  It works, but it does 
> not really solve anything. It also may mask potential other problems.
>
> Another alternative is to change clang driver and filter out unsupported 
> options so they are not passed to cc1. That will also work, but it looks 
> wrong, because now we have two patches that effectively cancel each other for 
> no observable benefit.
>
> Third option is to grow a better way to specify target-specific 
> sub-compilation options and then consider fancy debug flags to be 
> attributable to host compilation only. Anything beyond the "safe" subset, 
> would have to be specified explicitly.  This also sounds awkward -- I don't 
> really want to replicate bunch of options times number of GPUs I'm compiling 
> for. That may be alleviated by providing more coarse way to group options. 
> E.g. we could say "these are the options for *all* non-host compilations, and 
> here are few specifically for sm_XY". I think @echristo and I had discussed 
> something like this long time ago.


I think some amount of #3 is the most reasonable way forward here. Basically 
the compilation model of "pass the flags and hope they all work on all the 
targets that we're trying to compile for in this giant multiple target compile" 
seems to be a long tail of pain :) There are probably things we can do in the 
clang driver to make this a little easier, e.g. the "all non-host target 
options" flag you mentioned seems reasonable, but otherwise I think this is up 
to the user to split out flags that might not work.

Thoughts?


Repository:
  rC Clang

https://reviews.llvm.org/D49148



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


[PATCH] D50099: [DebugInfo][OpenCL] Address post-commit review of D49930

2018-08-05 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

Looks pretty good. Could you pass CGM in and just make the functions static I 
couldn't see any other class variables, but might have missed something. One 
inline comment as well.

-eric




Comment at: lib/CodeGen/CGDebugInfo.cpp:997
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagAppleBlock;
   unsigned LineNo = 0;
 

Just noticed that LineNo is 0... for the entire function.


https://reviews.llvm.org/D50099



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


[PATCH] D43045: Add NVPTX Support to ValidCPUList (enabling march notes)

2018-02-08 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM with an inline comment.




Comment at: include/clang/Basic/Cuda.h:49
   SM_72,
+  LAST,
 };

We have last, invalid, etc... maybe we should pick one among all the targets? :)



https://reviews.llvm.org/D43045



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


[PATCH] D43057: Add Rest of Targets Support to ValidCPUList (enabling march notes)

2018-02-08 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

I've glanced at this and didn't notice anything. I think any problems can be 
fixed in post-commit review if necessary.


https://reviews.llvm.org/D43057



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


[PATCH] D43095: Make attribute-target on a Definition-after-use update the LLVM attributes

2018-02-12 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:1318-1325
+llvm::AttrBuilder Attrs;
+if (GetCPUAndFeaturesAttributes(D, Attrs)) {
+  // We know that GetCPUAndFeaturesAttributes will always have the
+  // newest set, since it has the newest possible FunctionDecl, so the
+  // new ones should replace the old.
+  F->removeFnAttr("target-cpu");
+  F->removeFnAttr("target-features");

This feels awkward here. On a quick glance I'm not sure why we need this if 
we're adding above... is it possible to delay in such a way that we're not 
trying to add twice?


Repository:
  rC Clang

https://reviews.llvm.org/D43095



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


[PATCH] D43095: Make attribute-target on a Definition-after-use update the LLVM attributes

2018-02-12 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:1318-1325
+llvm::AttrBuilder Attrs;
+if (GetCPUAndFeaturesAttributes(D, Attrs)) {
+  // We know that GetCPUAndFeaturesAttributes will always have the
+  // newest set, since it has the newest possible FunctionDecl, so the
+  // new ones should replace the old.
+  F->removeFnAttr("target-cpu");
+  F->removeFnAttr("target-features");

erichkeane wrote:
> echristo wrote:
> > This feels awkward here. On a quick glance I'm not sure why we need this if 
> > we're adding above... is it possible to delay in such a way that we're not 
> > trying to add twice?
> I hadn't seen any.  The other call happens only on a declaration, this 
> happens only on definition.  There doesn't really seem to be a way to tell if 
> something is GOING to be defined later, since we often emit the declaration 
> immediately.
OK, sounds good. I didn't have any other feedback here :)


Repository:
  rC Clang

https://reviews.llvm.org/D43095



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


[PATCH] D43359: Clean up 'target' attribute diagnostics

2018-02-15 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

Seems ok here.


https://reviews.llvm.org/D43359



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


[PATCH] D43362: Simplify setting dso_local. NFC.

2018-02-22 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks for the cleanup.


https://reviews.llvm.org/D43362



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


[PATCH] D43514: Start settinng dso_local for COFF

2018-02-22 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

Some inline comments - I think this looks ok in general, but I'm curious about 
the answers to the questions/documentation bits inline.

Thanks!




Comment at: lib/CodeGen/CGDecl.cpp:257
 
+  setGVProperties(GV, &D);
+

If positioning is important here we should probably document it.



Comment at: lib/CodeGen/CodeGenModule.cpp:726
+  // Every other GV is local on COFF.
+  // Make an exception for windows OS in the triple: Some firmwares builds use
+  // *-win32-macho triples. This (accidentally?) produced windows relocations

"firmware"



Comment at: lib/CodeGen/CodeGenModule.cpp:733
+
   // Only handle ELF for now.
   if (!TT.isOSBinFormatELF())

This seems to need an update?



Comment at: lib/CodeGen/ItaniumCXXABI.cpp:1650
 
+  CGM.setGVProperties(VTable, RD);
+

Ditto with the moving comment from above.


https://reviews.llvm.org/D43514



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


[PATCH] D43514: Start settinng dso_local for COFF

2018-02-22 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments.



Comment at: lib/CodeGen/CodeGenModule.h:728
+  /// This must be called after dllimport/dllexport is set.
+  /// FIXME: should this set dllimport/dllexport instead?
   void setGVProperties(llvm::GlobalValue *GV, const NamedDecl *D) const;

Agreed? Maybe? Should the rest of the properties from above each call be set in 
this function?

Since you've got the decl it might be best to get as many of them in one place 
as possible?


https://reviews.llvm.org/D43514



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


[PATCH] D33448: [CodeGen] Add thumb-mode to function target-features for arm/thumb triples.

2017-05-24 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D33448#763411, @fhahn wrote:

> In https://reviews.llvm.org/D33448#762410, @echristo wrote:
>
> > I probably would have added this as a feature in ARMTargetInfo similar to 
> > CRC/soft-float/etc.
> >
> > Thoughts?
>
>
> Do you mean ARMTargetMachine::getSubtargetImpl 
> (https://github.com/llvm-mirror/llvm/blob/master/lib/Target/ARM/ARMTargetMachine.cpp#L305)
>  ?
>
> It seems like that function is only used by llc (and not llvm-as for example) 
> and at that stage it is too late to add thumb-mode, because llc operates on a 
> single module with a single target triple.


No, I meant somewhere around here: 
https://github.com/llvm-mirror/clang/blob/master/lib/Basic/Targets.cpp#L5058 to 
define it and then in CGCall.cpp it'll get added along with the other 
target-features.

I think that'll enable you to handle -mthumb and -target in a fairly unified 
way?


https://reviews.llvm.org/D33448



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


[PATCH] D33448: [CodeGen] Add thumb-mode to target-features for arm/thumb triples.

2017-05-25 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

One minor nit and LGTM.

Thanks!

-eric




Comment at: lib/Basic/Targets.cpp:5353
 
+// enable or disable thumb-mode per function
+if (isThumb())

Minor nit: comments are full sentences.


https://reviews.llvm.org/D33448



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


[PATCH] D33448: [CodeGen] Add thumb-mode to target-features for arm/thumb triples.

2017-05-26 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D33448#765749, @fhahn wrote:

> I'll hold off merging this patch until https://reviews.llvm.org/D33436 lands, 
> which fixes a problem with mixed ARM/Thumb codegen


OK. Commit at will :)

-eric


https://reviews.llvm.org/D33448



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


[PATCH] D33721: [ARM] Add support for target("arm") and target("thumb").

2017-05-31 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments.



Comment at: include/clang/Basic/Attr.td:1790-1794
+// target features respectively.
+if (Feature.compare("arm") == 0)
+  Ret.first.push_back("-thumb-mode");
+else if (Feature.compare("thumb") == 0)
+  Ret.first.push_back("+thumb-mode");

This is a little painful here - I wonder if we have access to TargetInfo or 
maybe translate it in the target specific area of the front end? I worry about 
this sort of thing getting unwieldy.

Thoughts?

Otherwise OK.


https://reviews.llvm.org/D33721



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


[PATCH] D33721: [ARM] Add support for target("arm") and target("thumb").

2017-06-01 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments.



Comment at: lib/Basic/Targets.cpp:5439-5442
+// [-|+]thumb-mode target features respectively.
+std::vector UpdatedFeaturesVec(FeaturesVec);
+for (auto &Feature : UpdatedFeaturesVec) {
+  if (Feature.compare("+arm") == 0)

Won't work below in handleTargetFeatures?


https://reviews.llvm.org/D33721



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


[PATCH] D33820: [PowerPC] Pass CPU to assembler with -no-integrated-as

2017-06-05 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

One inline comment otherwise LGTM




Comment at: lib/Driver/ToolChains/Gnu.cpp:681
 CmdArgs.push_back("-mppc");
-CmdArgs.push_back("-many");
+std::string CPU = getCPUName(Args, getToolChain().getTriple());
+CmdArgs.push_back(ppc::getPPCAsmModeForCPU(CPU));

Probably don't need to store it to a temporary since the other function is just 
returning a const char *.


Repository:
  rL LLVM

https://reviews.llvm.org/D33820



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


[PATCH] D33721: [ARM] Add support for target("arm") and target("thumb").

2017-06-05 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

Ah right. Thanks for looking.

LGTM.

-eric


https://reviews.llvm.org/D33721



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


[PATCH] D34022: Repair 2010-05-31-palignr.c test

2017-06-08 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

Thanks for the fix!

Do you need someone to commit it?


https://reviews.llvm.org/D34022



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


[PATCH] D34022: Repair 2010-05-31-palignr.c test

2017-06-08 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

Thanks!


https://reviews.llvm.org/D34022



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


[PATCH] D34304: Allow CompilerInvocations to generate .d files.

2017-06-16 Thread Eric Christopher via Phabricator via cfe-commits
echristo edited reviewers, added: bkramer; removed: echristo.
echristo added a comment.

Going to let Ben review this. I'd rather not pass the bool down and he might 
know a way to avoid that here by knowing the code more :)


https://reviews.llvm.org/D34304



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


[PATCH] D34425: Unified ARM logic for computing target ABI.

2017-06-21 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

This is OK once the dependent revision is approved.


https://reviews.llvm.org/D34425



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


[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

2018-01-03 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

Couple of inline comments, otherwise I'm pretty happy. I'd wait for an ack by 
Richard for this though.

-eric




Comment at: lib/CodeGen/CGBuiltin.cpp:7673
 
-Value *CodeGenFunction::EmitX86CpuInit() {
+Value *CodeGenFunction::EmitX86CpuInit(CGBuilderTy &Builder) {
   llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy,

Why do you need to pass in a Builder?



Comment at: lib/CodeGen/CodeGenFunction.cpp:2324
+  llvm::Triple::x86_64) &&
+ "Only implemented for x86 targets");
+

Can you get here via trying to compile code for another cpu?


https://reviews.llvm.org/D40819



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


[PATCH] D41837: Add Function multiversion to the release notes.

2018-01-08 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

I think you're missing that right now it's x86 only yes? :)

-eric


Repository:
  rC Clang

https://reviews.llvm.org/D41837



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


[PATCH] D43851: Start setting dllimport/dllexport in setGVProperties

2018-02-28 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

Seems reasonable to me.


https://reviews.llvm.org/D43851



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


[PATCH] D43248: [Attr] Fix parameter indexing for attributes

2018-03-13 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments.



Comment at: test/Sema/attr-print.cpp:3
 
+// This file is also used as input for %S/../Frontend/ast-attr.cpp.
+

Relatedly I don't think we use files as input files to other directories and I 
don't think we should really. What are you trying to test here? This breaks the 
hermeticness of each particular test directory.


Repository:
  rC Clang

https://reviews.llvm.org/D43248



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


[PATCH] D43248: [Attr] Fix parameter indexing for attributes

2018-03-13 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments.



Comment at: test/Sema/attr-print.cpp:3
 
+// This file is also used as input for %S/../Frontend/ast-attr.cpp.
+

jdenny wrote:
> echristo wrote:
> > Relatedly I don't think we use files as input files to other directories 
> > and I don't think we should really. What are you trying to test here? This 
> > breaks the hermeticness of each particular test directory.
> Grep for "%S/.." and you'll see that a few other tests do something like this 
> as well.
> 
> In test/Sema/attr-print.cpp, I am testing printing of attributes.  I chose to 
> put that there because of the existing attr-print.c there.
> 
> In test/Frontend/ast-attr.cpp, I am testing serialization of attributes.  I 
> chose to put that there because I see other -emit-ast tests there and 
> because, if I put it in test/Sema instead, I get the complaint "Do not use 
> the driver in Sema tests".
> 
> The same C++ code makes sense in both of these, but replicating it in two 
> files would worsen maintainability.
> 
> I could try to  combine into one file in, say, tests/Misc if that works.
> 
> I have no strong opinions on where these live.  Just for my own education, is 
> something real breaking right now because of their current locations?
> 
> Thanks.
Basically it's breaking an external build system (bazel) that has fairly 
distinct and specific dependency requirements and so layering and other 
dependencies are pretty entertaining.

Right now we avoid testing those particular tests and have TODOs of fixing them 
and the requirements to use a single directory and I was trying to avoid one 
more here.

All of that said I totally understand the desire to keep the maintenance burden 
minimized and an external build system shouldn't affect whether or not we do 
one particular thing or another - I was trying to get it written so that we 
could enable it without much undue burden. I'm uncertain if a comment of:

// If you change this file you should also change blah file.

or moving it to another directory where you can do both tests at the same time.


Repository:
  rC Clang

https://reviews.llvm.org/D43248



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


[PATCH] D43248: [Attr] Fix parameter indexing for attributes

2018-03-13 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D43248#1036426, @aaron.ballman wrote:

> In https://reviews.llvm.org/D43248#1036409, @jdenny wrote:
>
> > I'd prefer to move it than to expect people to obey such a comment.  Let's 
> > see what Aaron says.
>
>
> I have a slight preference for moving the tests now that I know they're 
> causing problems, unless that turns out to be overly onerous for some reason.
>
> Thank you, @echristo for pointing out the issues with the tests, I hadn't 
> considered that.


No worries :)

That said, please don't revert and reapply to move. Just a followup commit is 
just fine - and whenever you get a chance. (Though if you let me know when you 
do I'd appreciate it :)


Repository:
  rC Clang

https://reviews.llvm.org/D43248



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


[PATCH] D52920: Introduce code_model macros

2018-10-13 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

LGTM as well. Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D52920



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


[PATCH] D53248: [Driver] Support direct split DWARF emission for Fuchsia

2018-10-13 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

I'm ok with the "here are a bunch of fuchsia tests" file :)

-eric


Repository:
  rC Clang

https://reviews.llvm.org/D53248



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


[PATCH] D38061: Set AnonymousTagLocations false for ASTContext if column info is expected not to be used

2018-10-19 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a reviewer: aprantl.
echristo added a comment.

I think Adrian has looked at this more recently than I have. Adding him here.


Repository:
  rC Clang

https://reviews.llvm.org/D38061



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


[PATCH] D52441: [CodeGen] Update min-legal-vector width based on function argument and return types

2018-10-23 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D52441#1271545, @rnk wrote:

> In https://reviews.llvm.org/D52441#1258317, @craig.topper wrote:
>
> > Address Reid's comments. Add a comment with a list of all things that 
> > currently effect the vector width attribute emitted in IR.
> >
> > For inlining, we update the caller's attribute during merging to ensure it 
> > is at least as large as the callee that is being inlined. This is required 
> > for always_inline of the intrinsics. We probably want a way to limit 
> > inlining, but that would effect the inlining decision. If the decision has 
> > been made to inline we have to take the max.
> >
> > For LTO I don't have an answer. What do we do for things like target 
> > features and cpu today?
>
>
> I think your comments about the behavior w.r.t. inlining are enough to 
> describe what happens during LTO. I don't want to speak for Eric, but I think 
> you've answered his questions.


Yes, it'll be the same. As a note, the inlining widening must happen after the 
check for subtarget features.

Otherwise we talked about this at the conference and LGTM.


https://reviews.llvm.org/D52441



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


[PATCH] D53586: Implement Function Multiversioning for Non-ELF Systems.

2018-10-24 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

All of the target specific stuff looks fine to me. I'm going to defer to rnk 
about the windows side of things and aaron for the attributes.


https://reviews.llvm.org/D53586



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


[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.

2018-10-30 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D52296#1246142, @grimar wrote:

> In https://reviews.llvm.org/D52296#1243688, @echristo wrote:
>
> > In https://reviews.llvm.org/D52296#1241928, @probinson wrote:
> >
> > > Do we generate the .dwo file directly these days?  If not, I can imagine 
> > > wanting to avoid the overhead of the objcopy hack; as long as the linker 
> > > is smart enough not to bother with the .debug_*.dwo sections this seems 
> > > like a build-time win.
> >
> >
> > We do generate them generically with no objcopy hack.
>
>
> Eric, could you elaborate for me your position, please


We don't use objcopy on linux or fuschia to generate .dwo files.

> 
> 
>> As far as the standard text here, IMO it was just there in case people 
>> didn't have an objcopy around or don't want to split it.
> 
> Yeah, we do not want to split it and I see no other way to say clang to keep 
> them in a .o files rather than introducing the new flag.
>  Am I missing something?
> 
>> I'm not sure why we would want the ability. 
>>  That said, if we do I'd rather have it as dwarf5 without split-dwarf as an 
>> option rather than a -gsingle-file-split-dwarf option.
> 
> What do you mean as "dwarf5 without split-dwarf as an option" here? Do you 
> mean to do split-dwarf by default? It is orthogonal to what this patch does.

So, what are you trying to do here is, I guess, my other question.

Are you trying to take advantage of dwarf5 features, or are you just trying to 
have everything in one file for later splitting? Or something else? If you want 
to generate split dwarf (as opposed to a lot of dwarf5 features some of which 
are split dwarf) I'll bike shed that we need to come up with a better set of 
option naming here - I'd probably repurpose/alias -gsplit-dwarf and use -f 
options for whether or not to use specific features etc.

Can you elaborate on your motivations and what you're trying to do?

Thanks!


https://reviews.llvm.org/D52296



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


[PATCH] D53919: [X86] Don't allow illegal vector types to return by direct value on x86-64.

2018-11-05 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D53919#1282994, @hjl.tools wrote:

> In https://reviews.llvm.org/D53919#1282952, @efriedma wrote:
>
> > With both 3.3 and trunk (I don't have a 7.0 handy; I can build it if it 
> > would be helpful):
>
>
> Please try clang 2.6 on both testcases.


From the releases:

23 Oct 2009 2.6

... why would you care about a 9 year old version of clang?


Repository:
  rC Clang

https://reviews.llvm.org/D53919



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


[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.

2018-11-05 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D52296#1285328, @grimar wrote:

> In https://reviews.llvm.org/D52296#1284130, @probinson wrote:
>
> > In https://reviews.llvm.org/D52296#1283691, @grimar wrote:
> >
> > > Nice :) 
> > >  So seems the last unresolved question left is the naming of the new 
> > > option?
> > >  If we do not want to go with `-gsingle-file-split-dwarf` then what it 
> > > should be?
> > >
> > > What about `-fdebug-fission` as an alias for `-gsplit-dwarf`.
> > >  And `-fsingle-file-debug-fission` for the new option?
> > >
> > > Or, may be:
> > >
> > > `-fdebug-fission` for the new option and then
> > >  `-fdebug-fission -fdebug-split-dwo` would work together as 
> > > `-gsplit-dwarf`?
> >
> >
> > Only DWARF supports this, correct?
>
>
> I am not aware of any kind of debug information splitting except DWARF 
> splitting.
>
> > So I would suggest: `-fdwarf-fission[={split,single}]` where the parameter 
> > defaults to `split`. Is there any particular value in having two separate 
> > options?
>
> Probably not. `-fdwarf-fission[={split,single}]` sounds good for me.


Still not sure I understand the point, but this option and the alias sounds 
good to me.

Thanks.

-eric


https://reviews.llvm.org/D52296



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


[PATCH] D54243: DebugInfo: Add a driver flag for DWARF debug_ranges base address specifier use.

2018-11-08 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

I have no other bikeshed colors here. :)


Repository:
  rC Clang

https://reviews.llvm.org/D54243



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


[PATCH] D51554: [CUDA][OPENMP][NVPTX]Improve logic of the debug info support.

2018-11-08 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

The llvm backend patch here has discussion around debug info kinds that we 
should iron out first.




Comment at: lib/Driver/ToolChains/Cuda.cpp:292
+  bool IsDebugEnabled = !A || A->getOption().matches(options::OPT_O0) ||
+Args.hasFlag(options::OPT_cuda_noopt_device_debug,
+ options::OPT_no_cuda_noopt_device_debug,

Is this an nvcc compatibility flag?


Repository:
  rC Clang

https://reviews.llvm.org/D51554



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


[PATCH] D55016: Correctly support -shared-libgcc.

2018-12-03 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM.

-eric


Repository:
  rC Clang

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

https://reviews.llvm.org/D55016



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


[PATCH] D51554: [CUDA][OPENMP][NVPTX]Improve logic of the debug info support.

2018-12-05 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments.



Comment at: lib/Driver/ToolChains/Cuda.cpp:282-285
 enum DebugInfoKind {
-  NoDebug,   /// No debug info.
-  LineTableOnly, /// Line tables only.
-  FullDebug  /// Full debug info.
+  NoDebug, /// No debug info.
+  DebugDirectivesOnly, /// Line tables only.
+  FullDebug,   /// Full debug info.

This enum doesn't appear to be complete? Either way can you make it match the 
other and document what each thing means a bit more?



Comment at: lib/Driver/ToolChains/Cuda.cpp:289
 
 static DebugInfoKind mustEmitDebugInfo(const ArgList &Args) {
+  const Arg *A = Args.getLastArg(options::OPT_O_Group);

Please document this routine in prose.



Comment at: lib/Driver/ToolChains/Cuda.cpp:292
+  bool IsDebugEnabled = !A || A->getOption().matches(options::OPT_O0) ||
+Args.hasFlag(options::OPT_cuda_noopt_device_debug,
+ options::OPT_no_cuda_noopt_device_debug,

ABataev wrote:
> echristo wrote:
> > Is this an nvcc compatibility flag?
> No, nvcc uses different set of flags. It uses `-g` for the debug info for the 
> host code and `-G` for the device code. I'm not the original author of this 
> option. clang uses it to control emission of the debug info for the device.
> The bad thing about nvcc that it disables optimizations when `-G` is used. 
> Using this option we can use LLVM optimizations and disable the optimizations 
> only when we call `ptxas` tool.
OK.



Comment at: lib/Driver/ToolChains/Cuda.cpp:706-708
+void CudaToolChain::adjustDebugInfoKind(
+codegenoptions::DebugInfoKind &DebugInfoKind, const ArgList &Args) const {
+  switch (mustEmitDebugInfo(Args)) {

Is this really doing anything?


Repository:
  rC Clang

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

https://reviews.llvm.org/D51554



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


[PATCH] D61464: [RiscV] Typo in register aliases

2019-05-02 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM. Sorry I didn't notice this earlier.

-eric


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

https://reviews.llvm.org/D61464



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


[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

2019-05-06 Thread Eric Christopher via Phabricator via cfe-commits
echristo added reviewers: saugustine, cmatthews.
echristo added a comment.

Adding Sterling and Chris to this to take a look at the new layout :)


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

https://reviews.llvm.org/D59168



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


[PATCH] D61464: [RiscV] Typo in register aliases

2019-05-06 Thread Eric Christopher via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL360104: Fix typo in risc-v register aliases. (authored by 
echristo, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61464?vs=197864&id=198382#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61464

Files:
  cfe/trunk/lib/Basic/Targets/RISCV.cpp


Index: cfe/trunk/lib/Basic/Targets/RISCV.cpp
===
--- cfe/trunk/lib/Basic/Targets/RISCV.cpp
+++ cfe/trunk/lib/Basic/Targets/RISCV.cpp
@@ -31,7 +31,7 @@
   {{"zero"}, "x0"}, {{"ra"}, "x1"},  {{"sp"}, "x2"},   {{"gp"}, "x3"},
   {{"tp"}, "x4"},   {{"t0"}, "x5"},  {{"t1"}, "x6"},   {{"t2"}, "x7"},
   {{"s0"}, "x8"},   {{"s1"}, "x9"},  {{"a0"}, "x10"},  {{"a1"}, "x11"},
-  {{"a2"}, "x12"},  {{"a3"}, "x13"}, {{"a4"}, "x15"},  {{"a5"}, "x15"},
+  {{"a2"}, "x12"},  {{"a3"}, "x13"}, {{"a4"}, "x14"},  {{"a5"}, "x15"},
   {{"a6"}, "x16"},  {{"a7"}, "x17"}, {{"s2"}, "x18"},  {{"s3"}, "x19"},
   {{"s4"}, "x20"},  {{"s5"}, "x21"}, {{"s6"}, "x22"},  {{"s7"}, "x23"},
   {{"s8"}, "x24"},  {{"s9"}, "x25"}, {{"s10"}, "x26"}, {{"s11"}, "x27"},


Index: cfe/trunk/lib/Basic/Targets/RISCV.cpp
===
--- cfe/trunk/lib/Basic/Targets/RISCV.cpp
+++ cfe/trunk/lib/Basic/Targets/RISCV.cpp
@@ -31,7 +31,7 @@
   {{"zero"}, "x0"}, {{"ra"}, "x1"},  {{"sp"}, "x2"},   {{"gp"}, "x3"},
   {{"tp"}, "x4"},   {{"t0"}, "x5"},  {{"t1"}, "x6"},   {{"t2"}, "x7"},
   {{"s0"}, "x8"},   {{"s1"}, "x9"},  {{"a0"}, "x10"},  {{"a1"}, "x11"},
-  {{"a2"}, "x12"},  {{"a3"}, "x13"}, {{"a4"}, "x15"},  {{"a5"}, "x15"},
+  {{"a2"}, "x12"},  {{"a3"}, "x13"}, {{"a4"}, "x14"},  {{"a5"}, "x15"},
   {{"a6"}, "x16"},  {{"a7"}, "x17"}, {{"s2"}, "x18"},  {{"s3"}, "x19"},
   {{"s4"}, "x20"},  {{"s5"}, "x21"}, {{"s6"}, "x22"},  {{"s7"}, "x23"},
   {{"s8"}, "x24"},  {{"s9"}, "x25"}, {{"s10"}, "x26"}, {{"s11"}, "x27"},
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.

2018-09-20 Thread Eric Christopher via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342667: r342177 introduced a hint in cases where an 
#included file is not found. It… (authored by echristo, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52280?vs=166184&id=166327#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52280

Files:
  lib/Lex/PPDirectives.cpp


Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
 
   // Check for likely typos due to leading or trailing non-isAlphanumeric
   // characters
+  StringRef OriginalFilename = Filename;
   if (!File) {
-StringRef OriginalFilename = Filename;
 while (!isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
@@ -1915,7 +1915,7 @@
 
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
 }
   }


Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
 
   // Check for likely typos due to leading or trailing non-isAlphanumeric
   // characters
+  StringRef OriginalFilename = Filename;
   if (!File) {
-StringRef OriginalFilename = Filename;
 while (!isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
@@ -1915,7 +1915,7 @@
 
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52280: Don't trim non-alphanumeric characters in 'file not found' errors for include directives.

2018-09-20 Thread Eric Christopher via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342667: r342177 introduced a hint in cases where an 
#included file is not found. It… (authored by echristo, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52280?vs=166184&id=166326#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52280

Files:
  cfe/trunk/lib/Lex/PPDirectives.cpp


Index: cfe/trunk/lib/Lex/PPDirectives.cpp
===
--- cfe/trunk/lib/Lex/PPDirectives.cpp
+++ cfe/trunk/lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
 
   // Check for likely typos due to leading or trailing non-isAlphanumeric
   // characters
+  StringRef OriginalFilename = Filename;
   if (!File) {
-StringRef OriginalFilename = Filename;
 while (!isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
@@ -1915,7 +1915,7 @@
 
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
 }
   }


Index: cfe/trunk/lib/Lex/PPDirectives.cpp
===
--- cfe/trunk/lib/Lex/PPDirectives.cpp
+++ cfe/trunk/lib/Lex/PPDirectives.cpp
@@ -1887,8 +1887,8 @@
 
   // Check for likely typos due to leading or trailing non-isAlphanumeric
   // characters
+  StringRef OriginalFilename = Filename;
   if (!File) {
-StringRef OriginalFilename = Filename;
 while (!isAlphanumeric(Filename.front())) {
   Filename = Filename.drop_front();
 }
@@ -1915,7 +1915,7 @@
 
   // If the file is still not found, just go with the vanilla diagnostic
   if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
+Diag(FilenameTok, diag::err_pp_file_not_found) << OriginalFilename
<< FilenameRange;
 }
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.

2018-09-24 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D52296#1241928, @probinson wrote:

> Do we generate the .dwo file directly these days?  If not, I can imagine 
> wanting to avoid the overhead of the objcopy hack; as long as the linker is 
> smart enough not to bother with the .debug_*.dwo sections this seems like a 
> build-time win.


We do generate them generically with no objcopy hack.

As far as the standard text here, IMO it was just there in case people didn't 
have an objcopy around or don't want to split it. I'm not sure why we would 
want the ability. That said, if we do I'd rather have it as dwarf5 without 
split-dwarf as an option rather than a -gsingle-file-split-dwarf option.


https://reviews.llvm.org/D52296



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


[PATCH] D52296: [Clang] - Add -gsingle-file-split-dwarf option.

2018-09-24 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D52296#1241928, @probinson wrote:

> Do we generate the .dwo file directly these days?  If not, I can imagine 
> wanting to avoid the overhead of the objcopy hack; as long as the linker is 
> smart enough not to bother with the .debug_*.dwo sections this seems like a 
> build-time win.


We do generate them generically with no objcopy hack.

As far as the standard text here, IMO it was just there in case people didn't 
have an objcopy around or don't want to split it. I'm not sure why we would 
want the ability. That said, if we do I'd rather have it as dwarf5 without 
split-dwarf as an option rather than a -gsingle-file-split-dwarf option.


https://reviews.llvm.org/D52296



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


[PATCH] D52441: [CodeGen] Update min-legal-vector width based on function argument and return types

2018-10-01 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

Couple of comments/questions:

a) How do you want these attributes to be handled in merging/inlining? Also, 
are they a failure on module linking? In general, how do they work with LTO?
b) Could use some more comments when we're adding/merging the attributes in IR 
generation as well.


https://reviews.llvm.org/D52441



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


[PATCH] D36336: [X86] Add support for __builtin_cpu_init

2017-08-27 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

One inline comment, but go ahead and commit after fixing that up.




Comment at: lib/CodeGen/CGBuiltin.cpp:7292
const CallExpr *E) {
+  if (BuiltinID == X86::BI__builtin_cpu_init) {
+llvm::FunctionType *FTy = llvm::FunctionType::get(VoidTy,

I realize it makes sense to put this here because it doesn't require the setup, 
but let's go ahead and just put it in the switch statement below. I don't think 
avoiding the bit of setup/etc below is worth the inconsistency. :)


https://reviews.llvm.org/D36336



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


[PATCH] D36057: Use class to pass information about executable name

2017-08-28 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

Fine with me.

-eric


https://reviews.llvm.org/D36057



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


[PATCH] D37428: Debug info: Fixed faulty debug locations for attributed statements

2017-09-05 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments.



Comment at: test/CodeGen/debug-info-attributed-stmt.c:1
+// RUN: %clang_cc1 -triple x86_64-unk-unk -disable-llvm-passes 
-debug-info-kind=limited -emit-llvm %s -o - | FileCheck %s
+

Since we're not optimizing or generating code you should be able to remove the 
-disable-llvm-passes I believe.


https://reviews.llvm.org/D37428



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


[PATCH] D36431: Add powerpc64 to compiler-rt build infrastructure.

2017-09-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.

This is great with me.


https://reviews.llvm.org/D36431



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


[PATCH] D37656: [cfi] Set function attributes for __cfi_* functions.

2017-09-21 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:2917
+CodeGenFunction &CGF, llvm::Function *F,
+bool ForceThumb) {
+  StringRef TargetCPU = CGF.getTarget().getTargetOpts().CPU;

I don't think we should be force setting thumb anywhere, it should be handled 
on a function by function basis the way we do all of the target attribute stuff.



Comment at: clang/lib/CodeGen/CGExpr.cpp:2926-2935
+  Features.reserve(DefaultFeatures.size() + ForceThumb);
+  if (ForceThumb && (Triple.isARM() || Triple.isThumb())) {
+for (auto &S : DefaultFeatures)
+  if (S != "-thumb-mode" && S != "+thumb-mode")
+Features.push_back(S);
+Features.push_back("+thumb-mode");
+  } else {

All of this should be handled in lib/Basic/Targets/ARM.cpp 
ARMTargetInfo::initFeatureMap ?



Comment at: clang/lib/CodeGen/CGExpr.cpp:2961-2963
+  // attributes as SetLLVMFunctionAttributes sets. In particular, __cfi_check
+  // must use the default calling convention for the platform. ABI-changing
+  // flags like -mhard-float should not affect __cfi_check.

This is odd. Can you explain this a bit more?


https://reviews.llvm.org/D37656



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


[PATCH] D37656: [cfi] Set function attributes for __cfi_* functions.

2017-09-21 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:2917
+CodeGenFunction &CGF, llvm::Function *F,
+bool ForceThumb) {
+  StringRef TargetCPU = CGF.getTarget().getTargetOpts().CPU;

eugenis wrote:
> echristo wrote:
> > I don't think we should be force setting thumb anywhere, it should be 
> > handled on a function by function basis the way we do all of the target 
> > attribute stuff.
> We want this function to be Thumb even if the rest of the module is ARM. It 
> lets us use 2x less memory in the implementation of __cfi_slowpath here:
> https://clang.llvm.org/docs/ControlFlowIntegrityDesign.html#cfi-slowpath
> 
> This function does not have source, so it can not have target attributes, too.
> 
> Are you suggesting faking a piece of AST (FunctionDecl with attached 
> attributes) and then using the regular attribute setting code?
> 
> 
Aha. Didn't remember that part of the design.

And yeah, that sounds good. And it doesn't really matter if the function has 
any source code as to whether or not it has attributes. There's no inheritance 
structure going on here so every function should have them.



Comment at: clang/lib/CodeGen/CGExpr.cpp:2961-2963
+  // attributes as SetLLVMFunctionAttributes sets. In particular, __cfi_check
+  // must use the default calling convention for the platform. ABI-changing
+  // flags like -mhard-float should not affect __cfi_check.

eugenis wrote:
> echristo wrote:
> > This is odd. Can you explain this a bit more?
> __cfi_check is called from compiler-rt or libdl.so, so it can not have any 
> fancy calling convention. It must use what's standard on the platform.
> 
> OK, __cfi_check has no floating-point parameters, so -mhard-float would not 
> be a problem. 
Yeah, I think this falls in on the earlier comment. Should probably add the 
defaults to a bit of AST or something?


https://reviews.llvm.org/D37656



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


[PATCH] D62133: test/CodeGen/builtin-stackaddress.c duplicates test/CodeGen/2004-02-13-BuiltinFrameReturnAddress.c

2019-06-03 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

done thusly:

echristo@jhereg ~/s/llvm-project> git llvm push
Pushing 1 commit:

  d91813ca8c7 Remove test/CodeGen/builtin-stackaddress.c as it duplicates 
test/CodeGen/2004-02-13-BuiltinFrameReturnAddress.c.

Deleting   cfe/trunk/test/CodeGen/builtin-stackaddress.c
Committing transaction...
Committed revision 362462.
Committed d91813ca8c7 to svn.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62133



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


[PATCH] D62133: test/CodeGen/builtin-stackaddress.c duplicates test/CodeGen/2004-02-13-BuiltinFrameReturnAddress.c

2019-06-03 Thread Eric Christopher via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL362462: Remove test/CodeGen/builtin-stackaddress.c as it 
duplicates (authored by echristo, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62133?vs=200238&id=202819#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62133

Files:
  cfe/trunk/test/CodeGen/builtin-stackaddress.c


Index: cfe/trunk/test/CodeGen/builtin-stackaddress.c
===
--- cfe/trunk/test/CodeGen/builtin-stackaddress.c
+++ cfe/trunk/test/CodeGen/builtin-stackaddress.c
@@ -1,9 +0,0 @@
-// RUN: %clang_cc1 -emit-llvm < %s | grep "llvm.returnaddress"
-// RUN: %clang_cc1 -emit-llvm < %s | grep "llvm.frameaddress"
-void* a(unsigned x) {
-return __builtin_return_address(0);
-}
-
-void* c(unsigned x) {
-return __builtin_frame_address(0);
-}


Index: cfe/trunk/test/CodeGen/builtin-stackaddress.c
===
--- cfe/trunk/test/CodeGen/builtin-stackaddress.c
+++ cfe/trunk/test/CodeGen/builtin-stackaddress.c
@@ -1,9 +0,0 @@
-// RUN: %clang_cc1 -emit-llvm < %s | grep "llvm.returnaddress"
-// RUN: %clang_cc1 -emit-llvm < %s | grep "llvm.frameaddress"
-void* a(unsigned x) {
-return __builtin_return_address(0);
-}
-
-void* c(unsigned x) {
-return __builtin_frame_address(0);
-}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D64540: [CGDebugInfo] Simplfiy EmitFunctionDecl parameters, NFC

2019-07-11 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

Not a huge fan of boolean parameters like this, perhaps factor out the context 
as well into the caller and then we don't need it at all? Something else?


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

https://reviews.llvm.org/D64540



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


[PATCH] D65410: [PassManager] First Pass implementation at -O1 pass pipeline

2019-07-29 Thread Eric Christopher via Phabricator via cfe-commits
echristo created this revision.
echristo added reviewers: chandlerc, hfinkel.
Herald added subscribers: cfe-commits, jfb, dexonsmith, steven_wu, hiraditya, 
javed.absar, mcrosier, mehdi_amini.
Herald added projects: clang, LLVM.

As a follow-up to my initial mail to llvm-dev here's a first pass at the O1 
 described there.

Some rough internal testing using a bootstrap and test of clang has shown a 
combined build and test time for clang with nearly equivalent performance to O3 
 and quite a speedup over O0 - it's 
currently a little slower than the existing O1 
, likely due to the clang+llvm 
testsuite use of the same binaries many times rather than a few for individual 
tests. Build time is a bit better. For a larger build and smaller test time 
(think a couple of unittests), this is a bit better than either O3 
, O0, or O1 
. Overall binary size drops 
significantly compared to O0.

This change doesn't include any change to move from selection dag to fast isel 
and that will come with other numbers that should help inform that decision. I 
also haven't done any real debuggability studies with this pipeline yet, I 
wanted to get the initial start done so that people could see it and we could 
start tweaking after.

Test updates: Outside of the newpm tests most of the updates are coming from 
either optimization passes not run anymore (and without a compelling argument 
at the moment) that were largely used for canonicalization in clang.

Original post:

http://lists.llvm.org/pipermail/llvm-dev/2019-April/131494.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65410

Files:
  clang/test/CodeGen/2008-07-30-implicit-initialization.c
  clang/test/CodeGen/arm-fp16-arguments.c
  clang/test/CodeGen/arm-vfp16-arguments2.cpp
  clang/test/CodeGenCXX/auto-var-init.cpp
  clang/test/CodeGenCXX/stack-reuse.cpp
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/test/Feature/optnone-opt.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Transforms/MemCpyOpt/lifetime.ll
  llvm/test/Transforms/PhaseOrdering/simplifycfg-options.ll

Index: llvm/test/Transforms/PhaseOrdering/simplifycfg-options.ll
===
--- llvm/test/Transforms/PhaseOrdering/simplifycfg-options.ll
+++ llvm/test/Transforms/PhaseOrdering/simplifycfg-options.ll
@@ -7,7 +7,7 @@
 
 define i1 @PR33605(i32 %a, i32 %b, i32* %c) {
 ; ALL-LABEL: @PR33605(
-; ALL-NEXT:  for.body:
+; ALL-NEXT:  entry:
 ; ALL-NEXT:[[OR:%.*]] = or i32 [[B:%.*]], [[A:%.*]]
 ; ALL-NEXT:[[ARRAYIDX:%.*]] = getelementptr inbounds i32, i32* [[C:%.*]], i64 1
 ; ALL-NEXT:[[TMP0:%.*]] = load i32, i32* [[ARRAYIDX]], align 4
@@ -18,7 +18,7 @@
 ; ALL-NEXT:tail call void @foo()
 ; ALL-NEXT:br label [[IF_END]]
 ; ALL:   if.end:
-; ALL-NEXT:[[CHANGED_1_OFF0:%.*]] = phi i1 [ true, [[IF_THEN]] ], [ false, [[FOR_BODY:%.*]] ]
+; ALL-NEXT:[[CHANGED_1_OFF0:%.*]] = phi i1 [ true, [[IF_THEN]] ], [ false, [[ENTRY:%.*]] ]
 ; ALL-NEXT:[[TMP1:%.*]] = load i32, i32* [[C]], align 4
 ; ALL-NEXT:[[CMP_1:%.*]] = icmp eq i32 [[OR]], [[TMP1]]
 ; ALL-NEXT:br i1 [[CMP_1]], label [[IF_END_1:%.*]], label [[IF_THEN_1:%.*]]
Index: llvm/test/Transforms/MemCpyOpt/lifetime.ll
===
--- llvm/test/Transforms/MemCpyOpt/lifetime.ll
+++ llvm/test/Transforms/MemCpyOpt/lifetime.ll
@@ -1,4 +1,4 @@
-; RUN: opt < %s -O1 -S | FileCheck %s
+; RUN: opt < %s -O2 -S | FileCheck %s
 
 ; performCallSlotOptzn in MemCpy should not exchange the calls to
 ; @llvm.lifetime.start and @llvm.memcpy.
Index: llvm/test/Other/new-pm-thinlto-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-defaults.ll
@@ -108,13 +108,28 @@
 ; CHECK-O3-NEXT: Running pass: ArgumentPromotionPass
 ; CHECK-O-NEXT: Running pass: CGSCCToFunctionPassAdaptor<{{.*}}PassManager{{.*}}>
 ; CHECK-O-NEXT: Starting llvm::Function pass manager run.
-; CHECK-O-NEXT: Running pass: SROA
+; CHECK-O2-NEXT: Running pass: SROA
+; CHECK-O3-NEXT: Running pass: SROA
+; CHECK-Os-NEXT: Running pass: SROA
+; CHECK-Oz-NEXT: Running pass: SROA
 ; CHECK-O-NEXT: Running pass: EarlyCSEPass
 ; CHECK-O-NEXT: Running analysis: MemorySSAAnalysis
-; CHECK-O-NEXT: Running pass: SpeculativeExecutionPass
-; CHECK-O-NEXT: Running pass: JumpThreadingPass
-; CHECK-O-NEXT: Running analysis: LazyValueAnalysis
-; CHECK-O-NEXT: Running pass: CorrelatedValuePropagationPass
+; CHECK-O2-NEXT: Running pass: SpeculativeExecutionPass
+; CHECK-O3-NEXT: Running pass: SpeculativeExecutionPass
+; C

[PATCH] D65410: [PassManager] First Pass implementation at -O1 pass pipeline

2019-08-19 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In D65410#1613555 , @hfinkel wrote:

> Thanks for starting on this. Can you go ahead and replace the sroa calls with 
> mem2reg calls for `O1` and then see what that does to the performance? That 
> strikes me as a major change, but certainly one that potentially makes sense, 
> so I'd rather we go ahead and test it now before we make decisions about 
> other adjustments.


I'll give it a shot. I think we'll want it in the long run, but happy to run it 
through the performance blender just to get an idea of what we're getting for 
our complexity.

> FWIW, I thought that we might run InstCombine less often (or maybe replace it 
> with InstSimplify, in some places). Did you try that?

I haven't. It's one part of pass ordering that we'll want to look at, but I 
figured some optimizing here could happen later. Happy to try a few things 
though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65410



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


[PATCH] D62225: [clang][NewPM] Fixing -O0 tests that are broken under new PM

2019-05-21 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1104-1105
   // which is just that always inlining occurs.
-  MPM.addPass(AlwaysInlinerPass());
+  // We always pass false here since according to the legacy PM logic for
+  // enabling lifetime intrinsics, we should not be compiling with O0.
+  MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/false));

Can you elaborate more here? We do turn on the always inliner at O0 which makes 
this comment a bit confusing.



Comment at: clang/test/CodeGen/aarch64-neon-fma.c:235
 // CHECK: attributes #1 ={{.*}}"min-legal-vector-width"="128"
+// CHECK: attributes [[NOUNWIND_ATTR]] = { nounwind }

Do we really need to check for it or does the autogeneration of testcases do 
some of this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62225



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


[PATCH] D62133: test/CodeGen/builtin-stackaddress.c duplicates test/CodeGen/2004-02-13-BuiltinFrameReturnAddress.c

2019-05-28 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62133



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


[PATCH] D51554: [CUDA][OPENMP][NVPTX]Improve logic of the debug info support.

2018-12-11 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.

LGTM. I'm quite a bit happier with this now. Thanks for going through the back 
and forth.


Repository:
  rC Clang

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

https://reviews.llvm.org/D51554



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


[PATCH] D56044: [Driver] Support object files in addition to static and shared libraries in compiler-rt

2018-12-31 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

Is something passing compiler-rt in as a -r link output or something?


Repository:
  rC Clang

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

https://reviews.llvm.org/D56044



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


[PATCH] D56044: [Driver] Support object files in addition to static and shared libraries in compiler-rt

2018-12-31 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

OK, that makes sense. I'm not a huge fan of how this set of code looks, but it 
also seems unfair to require you to extensively refactor it for this.

Any issues with crtbegin/end and where people believe things should be 
implemented for any given OS is orthogonal to this.

Thanks!

-eric


Repository:
  rC Clang

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

https://reviews.llvm.org/D56044



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


[PATCH] D56043: [Driver] Don't pass default value to getCompilerRTArgString

2018-12-31 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM, thanks.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56043



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


[PATCH] D56181: [CMake][Fuchsia] Include check-lld in the list of bootstrap targets

2019-01-01 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

This seems like the sort of thing that you can just commit in the future.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56181



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


[PATCH] D67627: Clang-format: Add Whitesmiths indentation style

2019-09-16 Thread Eric Christopher via Phabricator via cfe-commits
echristo added reviewers: mboehme, krasimir.
echristo added a comment.

Martin should know who should look at this... maybe Krasimir?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67627



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


[PATCH] D46410: [Target] Diagnose mismatch in required CPU for always_inline functions

2018-05-03 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

I like the idea of a front end warning, but had believed that a subset of cpu 
features should be allowed to be inlined into something that's a superset and 
it sounds like you don't agree? Your thoughts here?

-eric


https://reviews.llvm.org/D46410



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


[PATCH] D48100: Append new attributes to the end of an AttributeList.

2018-06-25 Thread Eric Christopher via Phabricator via cfe-commits
echristo added subscribers: dlj, echristo.
echristo added a comment.

I've added a couple of inline comments here - between this and the comments in 
the post-commit review from dlj it seems like we might want to revert this for 
now and figure out the best way forward.

Thanks!

-eric




Comment at: test/Sema/attr-micromips.c:9
 
-__attribute__((micromips,mips16)) void foo5();  // expected-error 
{{'micromips' and 'mips16' attributes are not compatible}} \
+__attribute__((micromips,mips16)) void foo5();  // expected-error {{'mips16' 
and 'micromips' attributes are not compatible}} \
 // expected-note {{conflicting 
attribute is here}}

This seems to reverse? What's going on here? There are other occurrences too.



Comment at: test/Sema/attr-target-mv.c:98
 int __attribute__((target("sse4.2"))) diff_cc(void);
-// expected-error@+1 {{multiversioned function declaration has a different 
calling convention}}
+// expected-error@+1 {{attribute 'target' multiversioning cannot be combined 
with other attributes}}
 __vectorcall int __attribute__((target("arch=sandybridge")))  diff_cc(void);

This appears to have broken a particular error message?


Repository:
  rC Clang

https://reviews.llvm.org/D48100



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


[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

2018-07-17 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

I think you should break it out on an option by option basis. Just warning on 
"non-standard" options doesn't really make sense.


Repository:
  rC Clang

https://reviews.llvm.org/D49148



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


[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

2018-07-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D49148#1166429, @ABataev wrote:

> In https://reviews.llvm.org/D49148#1165826, @echristo wrote:
>
> > I think you should break it out on an option by option basis. Just warning 
> > on "non-standard" options won't make as much sense to end users. Perhaps a 
> > "this option is unsupported on the target you're compiling for" etc.
> >
> > Thoughts?
>
>
>
>
> 1. I can split it, no problems.
> 2. Hmm, actually this what the warning already says. If the option is not 
> supported it says 'debug option '-xxx' is not supported for target 
> 'xxx-xxx-xxx''. It does not seem to me like a warning on non-standard option.


Let me try to elaborate a bit, I agree that I'm not very clear above.

I'm not a fan of the generic "non default debug options". It feels misleading. 
I think we probably want to separate it by "dwarf extensions", and "dwarf 
version".

As far as the error message itself: "debug option" sounds like an option to 
debug clang rather than a debug information option. Perhaps say "debug 
information option" rather than "debug option"?


Repository:
  rC Clang

https://reviews.llvm.org/D49148



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


[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

2018-07-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

It's usually preferred to submit patches with full context - it'll make this a 
bit easier if you do please?


Repository:
  rC Clang

https://reviews.llvm.org/D46230



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


[PATCH] D49148: [DEBUGINFO] Disable unsupported debug info options for NVPTX target.

2018-07-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D49148#1166859, @echristo wrote:

> In https://reviews.llvm.org/D49148#1166429, @ABataev wrote:
>
> > In https://reviews.llvm.org/D49148#1165826, @echristo wrote:
> >
> > > I think you should break it out on an option by option basis. Just 
> > > warning on "non-standard" options won't make as much sense to end users. 
> > > Perhaps a "this option is unsupported on the target you're compiling for" 
> > > etc.
> > >
> > > Thoughts?
> >
> >
> >
> >
> > 1. I can split it, no problems.
> > 2. Hmm, actually this what the warning already says. If the option is not 
> > supported it says 'debug option '-xxx' is not supported for target 
> > 'xxx-xxx-xxx''. It does not seem to me like a warning on non-standard 
> > option.
>
>
> Let me try to elaborate a bit, I agree that I'm not very clear above.
>
> I'm not a fan of the generic "non default debug options". It feels 
> misleading. I think we probably want to separate it by "dwarf extensions", 
> and "dwarf version".
>
> As far as the error message itself: "debug option" sounds like an option to 
> debug clang rather than a debug information option. Perhaps say "debug 
> information option" rather than "debug option"?


Summarizing offline (irc) conversation:

Let's keep the generic solution that Alexey was using here, but instead migrate 
it to a "supports debug flag" routine that uses enums to check for which things 
we support. This way targets that don't quite support various levels of debug 
info or extensions can define them for themselves and even check OS versions.

(This would probably have been useful for darwin in the past and still might 
be).


Repository:
  rC Clang

https://reviews.llvm.org/D49148



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


[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

2018-07-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.

LGTM.

-eric


Repository:
  rC Clang

https://reviews.llvm.org/D46230



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


[PATCH] D46230: For x86_64, gcc 7.2 under Amazon Linux AMI sets its paths to x86_64-amazon-linux

2018-07-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

In https://reviews.llvm.org/D46230#1166958, @gaijiading wrote:

> In https://reviews.llvm.org/D46230#1166919, @echristo wrote:
>
> > LGTM.
> >
> > -eric
>
>
> Hi Eric, I do not have commit access to trunk. Could you commit the change 
> for me? Thanks.


The binaries make that hard, can you email me a tarball please?


Repository:
  rC Clang

https://reviews.llvm.org/D46230



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


[PATCH] D68535: Fix loop unrolling initialization in the new pass manager

2019-10-04 Thread Eric Christopher via Phabricator via cfe-commits
echristo created this revision.
echristo added reviewers: chandlerc, hfinkel.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, mcrosier.
Herald added projects: clang, LLVM.

In the new pass manager use PTO.LoopUnrolling to determine when and how
we will unroll loops. Also comment a few occasions where we need to
know whether or not we're forcing the unwinder or not.

Testcase is in clang because it's controlling how the loop optimizer
is being set up and there's no other way to trigger the behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68535

Files:
  clang/test/Misc/loop-opt-setup.c
  llvm/lib/Passes/PassBuilder.cpp


Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -470,8 +470,8 @@
   if ((Phase != ThinLTOPhase::PreLink || !PGOOpt ||
PGOOpt->Action != PGOOptions::SampleUse) &&
   PTO.LoopUnrolling)
-LPM2.addPass(
-LoopFullUnrollPass(Level, false, PTO.ForgetAllSCEVInLoopUnroll));
+LPM2.addPass(LoopFullUnrollPass(Level, /*OnlyWhenForced=*/false,
+PTO.ForgetAllSCEVInLoopUnroll));
 
   for (auto &C : LoopOptimizerEndEPCallbacks)
 C(LPM2, Level);
@@ -950,13 +950,13 @@
   // combiner for cleanup here so that the unrolling and LICM can be pipelined
   // across the loop nests.
   // We do UnrollAndJam in a separate LPM to ensure it happens before unroll
-  if (EnableUnrollAndJam) {
+  if (EnableUnrollAndJam && PTO.LoopUnrolling) {
 OptimizePM.addPass(
 createFunctionToLoopPassAdaptor(LoopUnrollAndJamPass(Level)));
   }
-  if (PTO.LoopUnrolling)
-OptimizePM.addPass(LoopUnrollPass(
-LoopUnrollOptions(Level, false, PTO.ForgetAllSCEVInLoopUnroll)));
+  OptimizePM.addPass(LoopUnrollPass(
+  LoopUnrollOptions(Level, /*OnlyWhenForced=*/!PTO.LoopUnrolling,
+PTO.ForgetAllSCEVInLoopUnroll)));
   OptimizePM.addPass(WarnMissedTransformationsPass());
   OptimizePM.addPass(InstCombinePass());
   OptimizePM.addPass(RequireAnalysisPass());
Index: clang/test/Misc/loop-opt-setup.c
===
--- /dev/null
+++ clang/test/Misc/loop-opt-setup.c
@@ -0,0 +1,12 @@
+// RUN: %clang -O1 -fexperimental-new-pass-manager -fno-unroll-loops -S -o - 
%s -emit-llvm | FileCheck %s
+// RUN: %clang -O1 -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s
+extern int a[16];
+int b = 0;
+int foo(void) {
+#pragma unroll
+  for (int i = 0; i < 16; ++i)
+a[i] = b += 2;
+  return b;
+}
+// CHECK-NOT: br i1
+


Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -470,8 +470,8 @@
   if ((Phase != ThinLTOPhase::PreLink || !PGOOpt ||
PGOOpt->Action != PGOOptions::SampleUse) &&
   PTO.LoopUnrolling)
-LPM2.addPass(
-LoopFullUnrollPass(Level, false, PTO.ForgetAllSCEVInLoopUnroll));
+LPM2.addPass(LoopFullUnrollPass(Level, /*OnlyWhenForced=*/false,
+PTO.ForgetAllSCEVInLoopUnroll));
 
   for (auto &C : LoopOptimizerEndEPCallbacks)
 C(LPM2, Level);
@@ -950,13 +950,13 @@
   // combiner for cleanup here so that the unrolling and LICM can be pipelined
   // across the loop nests.
   // We do UnrollAndJam in a separate LPM to ensure it happens before unroll
-  if (EnableUnrollAndJam) {
+  if (EnableUnrollAndJam && PTO.LoopUnrolling) {
 OptimizePM.addPass(
 createFunctionToLoopPassAdaptor(LoopUnrollAndJamPass(Level)));
   }
-  if (PTO.LoopUnrolling)
-OptimizePM.addPass(LoopUnrollPass(
-LoopUnrollOptions(Level, false, PTO.ForgetAllSCEVInLoopUnroll)));
+  OptimizePM.addPass(LoopUnrollPass(
+  LoopUnrollOptions(Level, /*OnlyWhenForced=*/!PTO.LoopUnrolling,
+PTO.ForgetAllSCEVInLoopUnroll)));
   OptimizePM.addPass(WarnMissedTransformationsPass());
   OptimizePM.addPass(InstCombinePass());
   OptimizePM.addPass(RequireAnalysisPass());
Index: clang/test/Misc/loop-opt-setup.c
===
--- /dev/null
+++ clang/test/Misc/loop-opt-setup.c
@@ -0,0 +1,12 @@
+// RUN: %clang -O1 -fexperimental-new-pass-manager -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s
+// RUN: %clang -O1 -fno-unroll-loops -S -o - %s -emit-llvm | FileCheck %s
+extern int a[16];
+int b = 0;
+int foo(void) {
+#pragma unroll
+  for (int i = 0; i < 16; ++i)
+a[i] = b += 2;
+  return b;
+}
+// CHECK-NOT: br i1
+
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D68255: [X86] Remove AVX/AVX512 check from validateOperandSize, just always accept 512

2019-10-05 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

The idea was that we'd have a much more graceful error message as well as a way 
for the front end to do the diagnosis. The code is being called out of 
SemaStmtAsm.cpp and I'm pretty sure we can probably accumulate the target 
attributes there?

-eric


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

https://reviews.llvm.org/D68255



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


[PATCH] D66795: [Mips] Use appropriate private label prefix based on Mips ABI

2019-10-21 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment.

I kinda want the option to be encoded in the triple for earlier testing of 
linking issues, but for now this is probably OK.


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

https://reviews.llvm.org/D66795



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


  1   2   3   >