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

[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] D56044: [Driver] Support object files in addition to static and shared libraries in compiler-rt

[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] D56181: [CMake][Fuchsia] Include check-lld in the list of bootstrap targets

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: mcgrathr, jakehehrlich, juliehockett, leonardchan.
Herald added subscribers: cfe-commits, mgorny.

This allows running lld tests when doing 2-stage toolchain build.


Repository:
  rC Clang

https://reviews.llvm.org/D56181

Files:
  clang/cmake/caches/Fuchsia.cmake


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -82,14 +82,17 @@
   check-all
   check-llvm
   check-clang
+  check-lld
   llvm-config
   test-suite
   test-depends
   llvm-test-depends
   clang-test-depends
+  lld-test-depends
   distribution
   install-distribution
   install-distribution-stripped
+  install-distribution-toolchain
   clang CACHE STRING "")
 
 get_cmake_property(variableNames VARIABLES)


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -82,14 +82,17 @@
   check-all
   check-llvm
   check-clang
+  check-lld
   llvm-config
   test-suite
   test-depends
   llvm-test-depends
   clang-test-depends
+  lld-test-depends
   distribution
   install-distribution
   install-distribution-stripped
+  install-distribution-toolchain
   clang CACHE STRING "")
 
 get_cmake_property(variableNames VARIABLES)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r350143 - Add vtable anchor to classes.

[David Blaikie via cfe-commits]

While I realize it's in the coding standard - is there any particular other
motivation for this? (Given you've been doing layering cleanup - I'm
wondering fi this is an interesting workaround for some layering problems,
for instance?)

On Sat, Dec 29, 2018 at 1:05 PM Richard Trieu via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rtrieu
> Date: Fri Dec 28 18:02:30 2018
> New Revision: 350143
>
> URL: http://llvm.org/viewvc/llvm-project?rev=350143&view=rev
> Log:
> Add vtable anchor to classes.
>
> Modified:
> cfe/trunk/include/clang/AST/DeclCXX.h
> cfe/trunk/include/clang/AST/DeclTemplate.h
> cfe/trunk/include/clang/Lex/ModuleMap.h
> cfe/trunk/lib/AST/DeclCXX.cpp
> cfe/trunk/lib/AST/DeclTemplate.cpp
> cfe/trunk/lib/Lex/ModuleMap.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=350143&r1=350142&r2=350143&view=diff
>
> ==
> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
> +++ cfe/trunk/include/clang/AST/DeclCXX.h Fri Dec 28 18:02:30 2018
> @@ -3918,6 +3918,7 @@ class MSPropertyDecl : public Declarator
>: DeclaratorDecl(MSProperty, DC, L, N, T, TInfo, StartL),
>  GetterId(Getter), SetterId(Setter) {}
>
> +  void anchor() override;
>  public:
>friend class ASTDeclReader;
>
>
> Modified: cfe/trunk/include/clang/AST/DeclTemplate.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclTemplate.h?rev=350143&r1=350142&r2=350143&view=diff
>
> ==
> --- cfe/trunk/include/clang/AST/DeclTemplate.h (original)
> +++ cfe/trunk/include/clang/AST/DeclTemplate.h Fri Dec 28 18:02:30 2018
> @@ -751,6 +751,7 @@ class RedeclarableTemplateDecl : public
>  return getMostRecentDecl();
>}
>
> +  void anchor() override;
>  protected:
>template  struct SpecEntryTraits {
>  using DeclType = EntryType;
>
> Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=350143&r1=350142&r2=350143&view=diff
>
> ==
> --- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
> +++ cfe/trunk/include/clang/Lex/ModuleMap.h Fri Dec 28 18:02:30 2018
> @@ -45,6 +45,8 @@ class SourceManager;
>  /// A mechanism to observe the actions of the module map parser as it
>  /// reads module map files.
>  class ModuleMapCallbacks {
> +  virtual void anchor();
> +
>  public:
>virtual ~ModuleMapCallbacks() = default;
>
>
> Modified: cfe/trunk/lib/AST/DeclCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=350143&r1=350142&r2=350143&view=diff
>
> ==
> --- cfe/trunk/lib/AST/DeclCXX.cpp (original)
> +++ cfe/trunk/lib/AST/DeclCXX.cpp Fri Dec 28 18:02:30 2018
> @@ -2910,6 +2910,8 @@ void DecompositionDecl::printName(llvm::
>os << ']';
>  }
>
> +void MSPropertyDecl::anchor() {}
> +
>  MSPropertyDecl *MSPropertyDecl::Create(ASTContext &C, DeclContext *DC,
> SourceLocation L, DeclarationName
> N,
> QualType T, TypeSourceInfo *TInfo,
>
> Modified: cfe/trunk/lib/AST/DeclTemplate.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclTemplate.cpp?rev=350143&r1=350142&r2=350143&view=diff
>
> ==
> --- cfe/trunk/lib/AST/DeclTemplate.cpp (original)
> +++ cfe/trunk/lib/AST/DeclTemplate.cpp Fri Dec 28 18:02:30 2018
> @@ -149,6 +149,8 @@ void *allocateDefaultArgStorageChain(con
>  // RedeclarableTemplateDecl Implementation
>
>  
> //===--===//
>
> +void RedeclarableTemplateDecl::anchor() {}
> +
>  RedeclarableTemplateDecl::CommonBase
> *RedeclarableTemplateDecl::getCommonPtr() const {
>if (Common)
>  return Common;
>
> Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=350143&r1=350142&r2=350143&view=diff
>
> ==
> --- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
> +++ cfe/trunk/lib/Lex/ModuleMap.cpp Fri Dec 28 18:02:30 2018
> @@ -54,6 +54,8 @@
>
>  using namespace clang;
>
> +void ModuleMapCallbacks::anchor() {}
> +
>  void ModuleMap::resolveLinkAsDependencies(Module *Mod) {
>auto PendingLinkAs = PendingLinkAsModule.find(Mod->Name);
>if (PendingLinkAs != PendingLinkAsModule.end()) {
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
__

r350182 - Make clearer which clang::type subclasses have visualizers

[Mike Spertus via cfe-commits]

Author: mps
Date: Mon Dec 31 15:01:34 2018
New Revision: 350182

URL: http://llvm.org/viewvc/llvm-project?rev=350182&view=rev
Log:
Make clearer which clang::type subclasses have visualizers

Modified:
cfe/trunk/utils/ClangVisualizers/clang.natvis

Modified: cfe/trunk/utils/ClangVisualizers/clang.natvis
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/ClangVisualizers/clang.natvis?rev=350182&r1=350181&r2=350182&view=diff
==
--- cfe/trunk/utils/ClangVisualizers/clang.natvis (original)
+++ cfe/trunk/utils/ClangVisualizers/clang.natvis Mon Dec 31 15:01:34 2018
@@ -38,7 +38,7 @@ For later versions of Visual Studio, no
 {*(clang::PackExpansionType *)this}
 {*(clang::LocInfoType *)this}
 {*this,view(poly)}
-{*this,view(cmn)} 
+No visualizer yet for 
{(clang::Type::TypeClass)TypeBits.TC,en}Type 
 {*this,view(cmn)}  {{{*this,view(poly)}}}
 
   (clang::Type::TypeClass)TypeBits.TC


___
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

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

I'd be happy to abandon this change if we decide to not move forward with 
D28791 . I've made that change a parent 
revision of this change (although they don't necessarily have to land in that 
order) to make it obvious.


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

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

This patch attempts to add support for .o/object files from compiler-rt and 
right now there are no users and the only potential ones are crt* according to 
my guess.


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

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

I'd suggest moving this discussion to D28791  
since your comments seem to be more related to the content of that change, not 
the content of this patch. I'm not trying to bypass the discussion in D28791 
, I'm just trying to split a rather large 
patch into smaller more easily reviewable chunks.


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

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a reviewer: joerg.
krytarowski added a comment.

Adding joerg who raised concerns in the original change and is bypassed with 
this change.


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

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

I don't know the original rationale of GNU.. but the final result (probably not 
out of interest of FSF) is that it's a vendor lock of glibc&gcc.

Please implement your crt* in your libc.


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

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

Sorry, but this rather belongs to libc(abi) of Fuchsia. Why to push it into 
LLVM?


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

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

This is needed for `crtbegin.o`/`crtend.o`, I originally implemented it as part 
of D28791  but that change is already pretty 
big so this is an attempt to split independent parts that could land separately 
into smaller changes.


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] D56134: [AST] Store some data of CXXNewExpr as trailing objects

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

LGTM.




Comment at: include/clang/AST/ExprCXX.h:1950
   /// The allocated type-source information, as written in the source.
   TypeSourceInfo *AllocatedTypeInfo;
 

If you ever want a somewhat bigger refactoring project that might provide major 
wins on C++ code, one idea I've had for awhile is to largely eliminate 
`TypeSourceInfo` from the AST in favor of storing a `TypeLoc` in most places.  
A TSI is basically a TL that stores the location data in a trailing array; it 
was convenient as a way to retrofit type-location information into the AST 
without massive changes, and it saves a pointer (at the cost of an extra 
indirection) when the TSI can be shared between parent nodes.  But I think the 
TSI can rarely be shared between parent nodes (pretty much just template 
instantiation of a non-dependent type), and the structure forces the location 
data to be copied when the type changes (e.g. during template instantiation) 
even if the location data would be the same.  It also means we can't use a 
preallocated zero buffer in `getTrivialTypeSourceInfo` with a null 
`SourceLocation`, and instead we have to allocate and initialize an 
appropriately-sized zero'ed trailing array.

The win wouldn't be as large as it could be because `FunctionProtoTypeLoc` 
includes `ParmVarDecl`s, which get cloned during template instantiation and 
therefore would always require a new type-location buffer when e.g. 
instantiating a function temploid.  But it might still be quite significant.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56134



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

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[Justin Hibbits via Phabricator via cfe-commits]

jhibbits added a comment.

Hi vit,

I found what's going on with the "Cannot Select" error.

divdc3.c contains code that gets lowered to a libcall.  However this lowering 
doesn't go further to lower down to the legal operations permitted in this 
target.

The debug snippet is:

Legalizing: t38: f64 = fmaxnum t36, t37, 
/home/chmeee/freebsd/contrib/compiler-rt/
lib/builtins/divdc3.c:24:22

  Op: t36: f64 = fabs t79, /home/chmeee/freebsd/contrib/compiler-rt/lib/builtins

/divdc3.c:24:22

  Op: t37: f64 = fabs t80, /home/chmeee/freebsd/contrib/compiler-rt/lib/builtins

/divdc3.c:24:22
Trying to expand node
Cannot expand node
Trying to convert node to libcall
Creating new node: t99: i64 = bitcast t36, 
/home/chmeee/freebsd/contrib/compiler-r
t/lib/builtins/divdc3.c:24:22
Creating new node: t100: i32 = extract_element t99, Constant:i32<1>, 
/home/chmeee/
freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
Creating new node: t101: i32 = extract_element t99, Constant:i32<0>, 
/home/chmeee/
freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
Creating new node: t102: i64 = bitcast t37, 
/home/chmeee/freebsd/contrib/compiler-
rt/lib/builtins/divdc3.c:24:22
Creating new node: t103: i32 = extract_element t102, Constant:i32<1>, 
/home/chmeee
/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
Creating new node: t104: i32 = extract_element t102, Constant:i32<0>, 
/home/chmeee
/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
Creating new node: t105: ch,glue = callseq_start t0, TargetConstant:i32<8>, 
Target
Constant:i32<0>, 
/home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24
:22
Creating new node: t107: ch,glue = CopyToReg t105, Register:i32 $r3, t100, 
/home/c
hmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
Creating new node: t108: ch,glue = CopyToReg t107, Register:i32 $r4, t101, 
t107:1,
 /home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
Creating new node: t110: ch,glue = CopyToReg t108, Register:i32 $r5, t103, 
t108:1,
 /home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
Creating new node: t112: ch,glue = CopyToReg t110, Register:i32 $r6, t104, 
t110:1,
 /home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
Creating new node: t114: ch,glue = PPCISD::CALL t112, 
TargetExternalSymbol:i32'fma
x' [TF=1], Register:i32 $r3, Register:i32 $r4, Register:i32 $r5, Register:i32 
$r6,
 RegisterMask:Untyped, t112:1, 
/home/chmeee/freebsd/contrib/compiler-rt/lib/builti
ns/divdc3.c:24:22
Creating new node: t115: ch,glue = callseq_end t114, TargetConstant:i32<8>, 
Target
Constant:i32<0>, t114:1, 
/home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/div
dc3.c:24:22
Creating new node: t116: i32,ch,glue = CopyFromReg t115, Register:i32 $r3, 
t115:1,
 /home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
Creating new node: t117: i32,ch,glue = CopyFromReg t116:1, Register:i32 $r4, 
t116:
2, /home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
Creating new node: t118: i64 = build_pair t117, t116, 
/home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
Creating new node: t119: f64 = bitcast t118, 
/home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
Created libcall: t119: f64 = bitcast t118, 
/home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22
Successfully converted node to libcall
 ... replacing: t38: f64 = fmaxnum t36, t37, 
/home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22

  with:  t119: f64 = bitcast t118, 
/home/chmeee/freebsd/contrib/compiler-rt/lib/builtins/divdc3.c:24:22

The last 3 lines are the key.  It's converting the node to a bitcast libcall, 
which ends up yielding a 'f64 bitcast (i64 build_pair i32, i32)', instead of 
lowering further to 'f64 build_spe64 i32, i32'.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49754



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

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaOverload.cpp:9279
+(CandAS1 != LangAS::opencl_generic && CandAS1 != LangAS::Default))
+  return true;
+  }

Anastasia wrote:
> rjmccall wrote:
> > rjmccall wrote:
> > > This at least needs a comment explaining the rule you're trying to 
> > > implement.
> > Okay, thanks for the clarification.  I think this should just be part of 
> > `CompareImplicitConversionSequence`, right?  It seems to me that you should 
> > prefer e.g. `int __private *` -> `int __private *` over `int __generic *`  
> > as a normal argument conversion as well.
> > 
> > Also, can this be written in terms of `isAddressSpaceSupersetOf`?  I don't 
> > remember how `LangAS::Default` works in OpenCL C++ enough to understand how 
> > it fits in here.
> That's correct we should implement the same logic for the arguments too. I 
> will create a helper function or do you think we should just call 
> `CompareImplicitConversionSequence` on the method type too?
> 
> I think `isAddressSpaceSupersetOf` can't be used here because it determines 
> compatibility of address spaces. However, the logic we are expressing is for 
> the address space preference instead.
If I understand correctly, we already call `CompareImplicitConversionSequence` 
on the object-argument conversion above, as part of the `for (unsigned ArgIdx = 
StartArg; ArgIdx < NumArgs; ++ArgIdx) ` loop.

I think the address-space ordering rule can be reasonably based on 
compatibility.  In fact, I think it already is in our implementation.  The 
right rule is basically that (1) an exact match is better than a conversion and 
(2) a conversion to a subspace is better than a conversion to a superspace.  
You can think of this as modifying the "proper subset" rule of 
[over.ics.rank]p3.2.5, which in implementation terms means 
`Qualifiers::isMoreQualifiedThan`, which already ends up using 
`isAddressSpaceSupersetOf`.  So if this ranking isn't working right, it's 
probably because we're incorrectly fast-pathing based on CVR qualifiers 
somewhere, and in fact I can see a pretty suspicious check like that in 
`CompareQualificationConversions`.


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

https://reviews.llvm.org/D55850



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

[PATCH] D56066: [OpenCL] Address space for default class members

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGCall.cpp:78
+RecTy = Context.getAddrSpaceQualType(
+RecTy, MD->getTypeQualifiers().getAddressSpace());
   return Context.getPointerType(CanQualType::CreateUnsafe(RecTy));

Would you mind making a patch to rename this (and the method on 
`FunctionProtoType`) to something like `getMethodQualifiers`?  It can be a 
follow-up instead of blocking this.



Comment at: lib/Sema/SemaDeclCXX.cpp:12649
+ArgType = Context.getAddrSpaceQualType(ClassType, LangAS::opencl_generic);
+  ArgType = Context.getRValueReferenceType(ClassType);
 

You're implicitly dropping the address space qualifier here by going back to 
`ClassType`.



Comment at: lib/Sema/SemaInit.cpp:4539
+  if (InitCategory.isPRValue() || InitCategory.isXValue())
+T1Quals.removeAddressSpace();
+

I can understand why a pr-value wouldn't have an address space, but an 
x-value's address space is surely important.


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

https://reviews.llvm.org/D56066



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

[PATCH] D55948: Modify DeclaratorChuck::getFunction to use DeclSpec for qualifiers

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/clang/Sema/DeclSpec.h:1365
+AttributeFactory AttrFactory;
+MethodQualifiers = new DeclSpec(AttrFactory);
+  }

This `DeclSpec` ends up with a dangling reference to the `AttributeFactory`; I 
think you need to allocate them both when used.

Consider having this return a `DeclSpec &` and making it the primary way that 
clients get a mutable DS.



Comment at: include/clang/Sema/DeclSpec.h:1405
 SourceLocation getConstQualifierLoc() const {
-  return SourceLocation::getFromRawEncoding(ConstQualifierLoc);
+  return MethodQualifiers->getConstSpecLoc();
 }

The "if any" suggests that that this method (and below) can be called on a DS 
that doesn't have this qualifier.  If that's not actually true, you should 
assert that `MethodQualifiers` exists and change the comment.



Comment at: include/clang/Sema/DeclSpec.h:1442
+ (MethodQualifiers->getTypeQualifiers() & Qualifiers::Restrict);
+}
+

It might be reasonable to just remove these accessors and make more things 
query the DS if it exists.



Comment at: include/clang/Sema/DeclSpec.h:1606
  Declarator &TheDeclarator,
+ DeclSpec& MethodQualifiers,
  TypeResult TrailingReturnType =

Spacing



Comment at: lib/Parse/ParseDeclCXX.cpp:2352
 FixItHint Insertion;
 if (DS.getTypeQualifiers() & TypeQual) {
+  if (!(Function.MethodQualifiers &&

Since you're touching this code anyway, please make this an early return 
(before the declaration of `Insertion`) and de-indent the rest of the helper.



Comment at: lib/Parse/ParseDeclCXX.cpp:2354
+  if (!(Function.MethodQualifiers &&
+(Function.MethodQualifiers->getTypeQualifiers() & TypeQual))) {
 std::string Name(FixItName);

Either `MethodQualifiers` already exists (because it has the qualifier) or 
you'll be creating it (to add the qualifier); might as well force it to exist 
eagerly and simplify the logic here and below.



Comment at: lib/Parse/ParseDeclCXX.cpp:2368
 }
   };
+  DeclSpecCheck(DeclSpec::TQ_const, "const", DS.getConstSpecLoc());

Please remove this dead semicolon since you're touching this code anyway.



Comment at: lib/Sema/DeclSpec.cpp:175
  Declarator &TheDeclarator,
+ DeclSpec& MethodQualifiers,
  TypeResult TrailingReturnType) {

Consider taking this as a pointer just so that the various places that 
construct one of these when method qualifiers are impossible don't have to 
construct a DS unnecessarily.



Comment at: lib/Sema/SemaDeclCXX.cpp:8374
   Diag(D.getIdentifierLoc(), diag::err_invalid_qualified_destructor)
-<< "restrict" << SourceRange(D.getIdentifierLoc());
+  << "restrict" << SourceRange(D.getIdentifierLoc());
 D.setInvalidType();

Should there be a method on DS that iterates over the type qualifiers present?  
You could have it take an `llvm::function_ref` 
or something.


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

https://reviews.llvm.org/D55948



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:9702
 
+static bool RejectConstNotMutableType(Sema &SemaRef, ValueDecl *D,
+  OpenMPClauseKind CKind,

Functions must start from the small letters.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:9716
+  RD->hasDefinition() && RD->hasMutableFields())) {
+SemaRef.Diag(ELoc, SemaRef.getLangOpts().CPlusPlus
+   ? diag::err_omp_const_not_mutable_variable

I would check here for `RD` rather than `CPlusPlus`. `int` types cannot have 
mutable fields, for example.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56113



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D56113#1342878 , @jdenny wrote:

> Hi Alexey,
>
> > In D56113#1342076 , @jdenny wrote:
> > 
> >> In D56113#1341940 , @ABataev 
> >> wrote:
> >>
> >> > The patch does not seem quite correct. I committed another fix for this 
> >> > problem.
> >>
> >>
> >> Thanks for the fix, r350127, which does seem to address the use cases I 
> >> care about right now.
> >>
> >> However, you are still implementing predetermined shared for const 
> >> variables.  Assuming OpenMP 4.0 or later, that produces misleading 
> >> diagnostics, such as "error: shared variable cannot be lastprivate".  
> >> Moreover, default(none) doesn't have the expected effect on such 
> >> variables.  Then again, neither of these issues seems like a severe 
> >> problem, and your approach is more compliant with earlier OpenMP versions. 
> >>  Does all that match your view?
> > 
> > 
> > I think this is a different problem, which you can try to fix. You just 
> > need to remove the rule for the const variables and carefully update all 
> > the checks in clauses. Also, you will need to add special checks for the 
> > constant types in all private clauses.
> >  If you want, you can implement this.
>
> I thought that's what I did in the patch I submitted.  Would you please get 
> me started by showing me an example where it doesn't behave correctly?  I 
> might be able to find remaining cases from that.


I thought you kept the original message, just missed that. The original cause 
of the problem was a little bit different and it masked the real problem. You 
can go ahead with your patch, update it and resend for the review.

> 
> 
>> Also, please note, that this going to be a serie of the patch - the first 
>> with the predetermined rule removal and several others with the correct 
>> handling of the constant types in the private clauses.
> 
> I'm happy to break up the patch.  However, the first patch you describe would 
> leave the test suite failing (many diagnostics will be skipped), and the 
> remaining patches you describe would fix the test suite.  Is that ok?

Ok, let's not break it. But you will need another serie of patches for 
`reduction` and `linear` clauses to update their error messages.

 There is still the problem with the explicitly specified `shared` clause 
 on `target teams` directive, but I don't think that it must cause some 
 troubles. The variable still should not be transferred from device to the 
 host, so it is fine to pass by value into inner teams regions. Though, 
 generally speaking, it must be fixed.
>>> 
>>> Should there be a copy allocated on the device and shared among the teams?  
>>> By the way, removing const from the example doesn't avoid this bug, but 
>>> splitting `omp target teams` into two directives does avoid it.
>> 
>> Just like I said, this is not a real problem. Moreover, it saves us one 
>> register for one extra load operation. So, I don't think this is a real 
>> problem and really requires a fix.
> 
> Sorry, I misunderstood your first response.  When I try an atomic write to 
> the shared variable, the difference between the uncombined and combined 
> target teams directive affects functionality when targeting multicore.  Does 
> that matter?

Yes, I'm aware if this problem. But I don't think it is very important and can 
cause serious problems in user code. If you think that it is worth to fix this 
prblem, you can go ahead and fix it.

> Thanks.



In D56113#1342878 , @jdenny wrote:

> Hi Alexey,
>
> > In D56113#1342076 , @jdenny wrote:
> > 
> >> In D56113#1341940 , @ABataev 
> >> wrote:
> >>
> >> > The patch does not seem quite correct. I committed another fix for this 
> >> > problem.
> >>
> >>
> >> Thanks for the fix, r350127, which does seem to address the use cases I 
> >> care about right now.
> >>
> >> However, you are still implementing predetermined shared for const 
> >> variables.  Assuming OpenMP 4.0 or later, that produces misleading 
> >> diagnostics, such as "error: shared variable cannot be lastprivate".  
> >> Moreover, default(none) doesn't have the expected effect on such 
> >> variables.  Then again, neither of these issues seems like a severe 
> >> problem, and your approach is more compliant with earlier OpenMP versions. 
> >>  Does all that match your view?
> > 
> > 
> > I think this is a different problem, which you can try to fix. You just 
> > need to remove the rule for the const variables and carefully update all 
> > the checks in clauses. Also, you will need to add special checks for the 
> > constant types in all private clauses.
> >  If you want, you can implement this.
>
> I thought that's what I did in the patch I submitted.  Would you please get 
> me started by showing 

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

Hi Alexey,

> In D56113#1342076 , @jdenny wrote:
> 
>> In D56113#1341940 , @ABataev wrote:
>>
>> > The patch does not seem quite correct. I committed another fix for this 
>> > problem.
>>
>>
>> Thanks for the fix, r350127, which does seem to address the use cases I care 
>> about right now.
>>
>> However, you are still implementing predetermined shared for const 
>> variables.  Assuming OpenMP 4.0 or later, that produces misleading 
>> diagnostics, such as "error: shared variable cannot be lastprivate".  
>> Moreover, default(none) doesn't have the expected effect on such variables.  
>> Then again, neither of these issues seems like a severe problem, and your 
>> approach is more compliant with earlier OpenMP versions.  Does all that 
>> match your view?
> 
> 
> I think this is a different problem, which you can try to fix. You just need 
> to remove the rule for the const variables and carefully update all the 
> checks in clauses. Also, you will need to add special checks for the constant 
> types in all private clauses.
>  If you want, you can implement this.

I thought that's what I did in the patch I submitted.  Would you please get me 
started by showing me an example where it doesn't behave correctly?  I might be 
able to find remaining cases from that.

> Also, please note, that this going to be a serie of the patch - the first 
> with the predetermined rule removal and several others with the correct 
> handling of the constant types in the private clauses.

I'm happy to break up the patch.  However, the first patch you describe would 
leave the test suite failing (many diagnostics will be skipped), and the 
remaining patches you describe would fix the test suite.  Is that ok?

>>> There is still the problem with the explicitly specified `shared` clause on 
>>> `target teams` directive, but I don't think that it must cause some 
>>> troubles. The variable still should not be transferred from device to the 
>>> host, so it is fine to pass by value into inner teams regions. Though, 
>>> generally speaking, it must be fixed.
>> 
>> Should there be a copy allocated on the device and shared among the teams?  
>> By the way, removing const from the example doesn't avoid this bug, but 
>> splitting `omp target teams` into two directives does avoid it.
> 
> Just like I said, this is not a real problem. Moreover, it saves us one 
> register for one extra load operation. So, I don't think this is a real 
> problem and really requires a fix.

Sorry, I misunderstood your first response.  When I try an atomic write to the 
shared variable, the difference between the uncombined and combined target 
teams directive affects functionality when targeting multicore.  Does that 
matter?

Thanks.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56113



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

[PATCH] D33499: [PPC] PPC32/Darwin ABI info

[Ken Cunningham via Phabricator via cfe-commits]

ken-cunningham-webuse added a comment.

I  found the relevant recent discussions on the mailing list, and the commits 
from August that started the Darwin PPC withdrawal from the source.

I have this patch in my local folder of llvm/darwinppc fixes. I suppose this 
ticket might be closed then?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D33499



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

[PATCH] D54408: [ASTMatchers] Add matchers available through casting to derived

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:651
+llvm::Optional>
+getNodeConstructorType(MatcherCtor targetCtor) {
+  const auto &ctors = RegistryData->nodeConstructors();

targetCtor -> TargetCtor



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:653
+  const auto &ctors = RegistryData->nodeConstructors();
+  auto it = llvm::find_if(
+  ctors, [targetCtor](const NodeConstructorMap::value_type &ctor) {

it -> It



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:654
+  auto it = llvm::find_if(
+  ctors, [targetCtor](const NodeConstructorMap::value_type &ctor) {
+return ctor.second.second == targetCtor;

ctor -> Ctor



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:657-659
+  if (it == ctors.end()) {
+return llvm::None;
+  }

Elide braces



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:683
+if (Scope == ExactOnly) {
+  if (Matcher.isConvertibleTo(StaticType, &Specificity,
+  &LeastDerivedKind) &&

Can combine with the previous `if` statement, I believe.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:689
+
+auto TypeForMatcherOpt = getNodeConstructorType(&Matcher);
+if (TypeForMatcherOpt &&

Don't use `auto` here please.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:692
+StaticType.isBaseOf(TypeForMatcherOpt->first)) {
+  auto Derived = getDerivedResults(TypeForMatcherOpt->first, Name);
+  Result.insert(Result.end(), Derived.begin(), Derived.end());

Don't use `auto` here please.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:705
+getDerivedResults(ast_type_traits::ASTNodeKind StaticType, StringRef Name) {
+  auto NestedResult = getMatchingMatchersImpl(StaticType, ExactOnly);
+

Don't use `auto` here please.



Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:711
+
+  for (const auto &item : NestedResult) {
+Result.emplace_back((Name + "(" + item.MatcherString + ")").str());

item -> Item


Repository:
  rC Clang

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

https://reviews.llvm.org/D54408



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

[PATCH] D56090: Add a matcher for members of an initializer list expression

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.

In D56090#1341023 , @hwright wrote:

> @lebedev.ri Where do the appropriate tests live?  (I couldn't find an obvious 
> subdirectory in `test/`)


This would live in `clang\unittests\ASTMatchers\ASTMatchersNarrowingTest.cpp`

> Where are the instructions for regenerating the documentation?

Run `clang\docs\tools\dump_ast_matchers.py` and it will generate the 
documentation for you.

You should also update Registry.cpp to list the new matcher.




Comment at: include/clang/ASTMatchers/ASTMatchers.h:3527
+  return N < Node.getNumInits() &&
+  InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder,
+   Builder);

I'm not certain we want the `IgnoreParenImpCasts()` here -- what if someone 
wants to match an initializer that uses one of those properties?


Repository:
  rC Clang

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

https://reviews.llvm.org/D56090



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

[PATCH] D55710: add pragmas to control Software Pipelining optimisation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The docs are getting much closer -- just a few grammatical things left to fix, 
I believe.




Comment at: include/clang/Basic/AttrDocs.td:2582
 specified for the subsequent loop. The directive allows vectorization,
-interleaving, and unrolling to be enabled or disabled. Vector width as well
-as interleave and unrolling count can be manually specified. See
-`language extensions
+interleaving, and unrolling to be enabled or disabled. Pipelining could be 
disabled.
+Vector width, interleave count, unrolling count, and the initiation interval 
for pipelining

Can you combine the new sentence with the previous one? `The directive allows 
vectorization, interleaving, pipelining, and unrolling to be enabled or 
disabled.`



Comment at: include/clang/Basic/AttrDocs.td:2657
+  is the number of cycles between two iterations of an unoptimized loop in
+  newly created schedule. New optimized loop is created such that a single 
iteration
+  of the loop executes in the same number of cycles as the initiation interval.

in newly created schedule -> in the newly created schedule

New optimized loop -> A new, optimized loop



Comment at: include/clang/Basic/AttrDocs.td:2662
+  ``#pragma clang loop pipeline and #pragma loop pipeline_initiation_interval``
+  could be used as hints for Software Pipelining optimization. The pragma is
+  placed immediately before a for, while, do-while, or c++11 range-based for

for Software Pipelining optimization. -> for the software pipelining 
optimization.



Comment at: include/clang/Basic/AttrDocs.td:2663
+  could be used as hints for Software Pipelining optimization. The pragma is
+  placed immediately before a for, while, do-while, or c++11 range-based for
+  loop.

or c++11 range-based for loop. -> or a C++11 range-based for loop.



Comment at: include/clang/Basic/AttrDocs.td:2666
+
+  Using ``#pragma clang loop pipeline(disable)`` avoids software pipelining
+  optimization. The disable state can only be specified:

avoids software -> avoids the software



Comment at: include/clang/Basic/AttrDocs.td:2678
+  the software pipeliner to try the specified initiation interval.
+  If schedule was found then the resulting loop iteration would have
+  specified cycle count. If schedule was not found then loop would stay

If schedule was found -> If a schedule was found



Comment at: include/clang/Basic/AttrDocs.td:2679
+  If schedule was found then the resulting loop iteration would have
+  specified cycle count. If schedule was not found then loop would stay
+  unchanged. The initiation interval must be a positive number

would have specified cycle count. -> would have the specified cycle count.

If schedule was not -> If a schedule was not
would stay -> remains


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

https://reviews.llvm.org/D55710



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

[PATCH] D56157: [sanitizer_common] Implement popen, popenve, pclose interceptors

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added inline comments.



Comment at: lib/esan/esan_interceptors.cpp:90
   } while (false)
+#define COMMON_INTERCEPTOR_PIPE_OPEN(ctx, file)
\
+  do { 
\

dvyukov wrote:
> The idea behind defining a no-op version in sanitizer_common_interceptors.inc 
> was exactly that tools that are not interested in it does not need to be 
> changed. So please remove this.
I would add an intermediate revision to drop redefinitions in this file.



Comment at: lib/tsan/rtl/tsan_interceptors.cc:2259
 
+#define COMMON_INTERCEPTOR_PIPE_OPEN(ctx, file) \
+  if (file) {   \

dvyukov wrote:
> An alternative would be to pass NULL to COMMON_INTERCEPTOR_FILE_OPEN and then 
> do:
> 
>   if (path)
> Acquire(thr, pc, File2addr(path))
> 
> Just to not multiply entities. But neither approach looks strictly better to 
> me, so I am not too strong about it.
Is `File2addr()` producing any useful result?

Reusing `COMMON_INTERCEPTOR_FILE_OPEN` looks fine.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D56157



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

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D56113#1342076 , @jdenny wrote:

> In D56113#1341940 , @ABataev wrote:
>
> > The patch does not seem quite correct. I committed another fix for this 
> > problem.
>
>
> Thanks for the fix, r350127, which does seem to address the use cases I care 
> about right now.
>
> However, you are still implementing predetermined shared for const variables. 
>  Assuming OpenMP 4.0 or later, that produces misleading diagnostics, such as 
> "error: shared variable cannot be lastprivate".  Moreover, default(none) 
> doesn't have the expected effect on such variables.  Then again, neither of 
> these issues seems like a severe problem, and your approach is more compliant 
> with earlier OpenMP versions.  Does all that match your view?
>
> > There is still the problem with the explicitly specified `shared` clause on 
> > `target teams` directive, but I don't think that it must cause some 
> > troubles. The variable still should not be transferred from device to the 
> > host, so it is fine to pass by value into inner teams regions. Though, 
> > generally speaking, it must be fixed.
>
> Should there be a copy allocated on the device and shared among the teams?  
> By the way, removing const from the example doesn't avoid this bug, but 
> splitting `omp target teams` into two directives does avoid it.




In D56113#1342076 , @jdenny wrote:

> In D56113#1341940 , @ABataev wrote:
>
> > The patch does not seem quite correct. I committed another fix for this 
> > problem.
>
>
> Thanks for the fix, r350127, which does seem to address the use cases I care 
> about right now.
>
> However, you are still implementing predetermined shared for const variables. 
>  Assuming OpenMP 4.0 or later, that produces misleading diagnostics, such as 
> "error: shared variable cannot be lastprivate".  Moreover, default(none) 
> doesn't have the expected effect on such variables.  Then again, neither of 
> these issues seems like a severe problem, and your approach is more compliant 
> with earlier OpenMP versions.  Does all that match your view?


I think this is a different problem, which you can try to fix. You just need to 
remove the rule for the const variables and carefully update all the checks in 
clauses. Also, you will need to add special checks for the constant types in 
all private clauses.
If you want, you can implement this.Also, please note, that this going to be a 
serie of the patch - the first with the predetermined rule removal and several 
others with the correct handling of the constant types in the private clauses.

> 
> 
>> There is still the problem with the explicitly specified `shared` clause on 
>> `target teams` directive, but I don't think that it must cause some 
>> troubles. The variable still should not be transferred from device to the 
>> host, so it is fine to pass by value into inner teams regions. Though, 
>> generally speaking, it must be fixed.
> 
> Should there be a copy allocated on the device and shared among the teams?  
> By the way, removing const from the example doesn't avoid this bug, but 
> splitting `omp target teams` into two directives does avoid it.

Just like I said, this is not a real problem. Moreover, it saves us one 
register for one extra load operation. So, I don't think this is a real problem 
and really requires a fix.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56113



___
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

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added a comment.

What's the rationale for this?


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

[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