rnk added a comment.
In https://reviews.llvm.org/D28365#640868, @hamzasood wrote:
> Ha, good point. Does that include the environment stuff in Command too or
> just the linker?
Yes, please make the Command environment changes as part of a separate patch
with the linker environment changes.
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D30947
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk updated this revision to Diff 91749.
rnk added a comment.
- Test that explicit casts suppress the warning
https://reviews.llvm.org/D30923
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
test/SemaCXX/warn-bitfield-en
rnk added a comment.
In https://reviews.llvm.org/D30923#700708, @hfinkel wrote:
> In https://reviews.llvm.org/D30923#700696, @rnk wrote:
>
> > Do you think it's worth indicating that the error can be suppressed with an
> > explicit cast, or is that wasted space?
>
>
> What might this look like?
rnk marked an inline comment as done.
rnk added a comment.
Cool, thanks for the reviews, this is definitely in better shape now. This is
off by default, so I'm going to commit and experiment with it on a few
codebases.
I'd still like to hear if either of the Richards have diagnostic wording
su
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297761: Warn on enum assignment to bitfields that can't fit
all values (authored by rnk).
Changed prior to commit:
https://reviews.llvm.org/D30923?vs=91749&id=91752#toc
Repository:
rL LLVM
https://r
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
Lgtm
https://reviews.llvm.org/D31162
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Repository:
rL LLVM
https://reviews.llvm.org/D31174
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailm
rnk added a comment.
I think you need some driver logic to make sure the xray lists show up in .d
files so that modifications to them trigger rebuilds in most build systems. See
the SanitizerArgs.cpp driver logic for this.
Comment at: include/clang/Basic/XRayFunctionFilter.h:
rnk added inline comments.
Comment at: lib/Basic/XRayFunctionFilter.cpp:21
+: AlwaysInstrument(
+ llvm::SpecialCaseList::createOrDie(AlwaysInstrumentPaths)),
+
NeverInstrument(llvm::SpecialCaseList::createOrDie(NeverInstrumentPaths)),
Hm, phab
rnk added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:2748
+if (const auto *XRayAttr =
+this->CurFuncDecl->getAttr()) {
+ if (XRayAttr->neverXRayInstrument())
Don't need `this->`
Comment at: lib/CodeGen/CGBuiltin
rnk added a comment.
Yep, thanks, looks good!
Repository:
rL LLVM
https://reviews.llvm.org/D31248
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: lib/CodeGen/CGCoroutine.cpp:225
+ void Emit(CodeGenFunction &CGF, Flags) override {
+CGF.EmitStmt(Deallocate);
+ }
This will be called twice
rnk added inline comments.
Comment at: lib/CodeGen/CGCoroutine.cpp:225
+ void Emit(CodeGenFunction &CGF, Flags) override {
+CGF.EmitStmt(Deallocate);
+ }
GorNishanov wrote:
> rnk wrote:
> > This will be called twice: once for a normal exit and once for exce
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: include/clang/Driver/XRayArgs.h:22
+class XRayArgs {
+ std::vector AlwaysInstrumenFiles;
+ std::vector NeverInstrumentFiles;
Any reason to omit
rnk added inline comments.
Comment at: cfe/trunk/include/clang/Basic/LangOptions.h:217
/// Adjust BinaryOperator::FPFeatures to match the bit-field size of this.
- unsigned fp_contract : 1;
+ LangOptions::FPContractModeKind fp_contract : 2;
};
Please do not
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: test/CodeGen/xray-instruction-threshold.cpp:11
+
+// CHECK-DAG: define i32 @_Z3foov() #[[THRESHOLD:[0-9]+]] {
+// CHECK-DAG: define i32 @_Z3barv() #[[NEVERATTR:[0-
rnk added inline comments.
Comment at: cfe/trunk/include/clang/Basic/LangOptions.h:217
/// Adjust BinaryOperator::FPFeatures to match the bit-field size of this.
- unsigned fp_contract : 1;
+ LangOptions::FPContractModeKind fp_contract : 2;
};
anemet wrote:
rnk created this revision.
Herald added a subscriber: mgorny.
mingw64 has lots of default libs that libc++ and its test programs
should link against.
With this patch, cmake now runs successfully with GCC on Windows.
https://reviews.llvm.org/D31518
Files:
libcxx/cmake/config-ix.cmake
Index:
This revision was automatically updated to reflect the committed changes.
Closed by commit rL299144: Try to fix the libcxx build with mingw64 (authored
by rnk).
Changed prior to commit:
https://reviews.llvm.org/D31518?vs=93568&id=93570#toc
Repository:
rL LLVM
https://reviews.llvm.org/D31518
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
Looks good to me
Comment at: lib/Sema/SemaDecl.cpp:6710-6711
+
+ // Return false if warning is ignored.
+ if (Diags.isIgnored(diag::warn_decl_shadow, R.getNameLoc()))
+re
rnk added inline comments.
Comment at: lib/Driver/ToolChains/Cuda.cpp:324
+ "CUDA toolchain not expected for an OpenMP host device.");
+ if (JA.isDeviceOffloading(Action::OFK_OpenMP)) {
+if (Output.isFilename()) {
This entire if block constructs a co
rnk added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:570
+llvm::AtomicOrdering::SequentiallyConsistent);
+llvm::Value *Shifted = Builder.CreateLShr(RMWI, Bit);
+llvm::Value *Truncated =
Can you comment that this shifts the relevant bit
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: lib/CodeGen/CGBuiltin.cpp:570
+llvm::AtomicOrdering::SequentiallyConsistent);
+llvm::Value *Shifted = Builder.CreateLShr(RMWI, Bit);
+llvm::Value *
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D31702
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk added inline comments.
Comment at: clang/tools/.gitignore:12
+#==#
+# The include-what-you-use project, for when building in-tree.
+include-what-you-use
LLVM doesn't currently have an
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
Looks good
https://reviews.llvm.org/D32014
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c
rnk added a comment.
In https://reviews.llvm.org/D32064#726861, @pcc wrote:
> I think it would be better to move this logic to the driver and have it pass
> in an `-mllvm` flag. The sanitizer passes should really be taking no
> arguments in the constructor like the other passes, so I don't want
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
Let's add it now. If we don't add this, some user will run into it first and
have to wait for a fix. If we add it and get the meaning wrong, we'll still
need to fix it and push a new release, so we'
rnk added inline comments.
Comment at: lib/Driver/SanitizerArgs.cpp:636
+ case llvm::Triple::COFF:
+return DataSections;
+ case llvm::Triple::ELF:
eugenis wrote:
> rnk wrote:
> > We can return true for COFF here. By adding a comdat during asan
> > instrume
This revision was automatically updated to reflect the committed changes.
Closed by commit rL300572: Remove unused varible (authored by rnk).
Changed prior to commit:
https://reviews.llvm.org/D32014?vs=95123&id=95602#toc
Repository:
rL LLVM
https://reviews.llvm.org/D32014
Files:
cfe/trunk
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm, thanks for the fix!
https://reviews.llvm.org/D28590
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman
rnk added inline comments.
Comment at: lib/Driver/MSVCToolChain.cpp:34
+ #if 0
+#define USE_VS_SETUP_CONFIG
+ #endif
What are the outstanding issues preventing you from enabling this by default?
Comment at: lib/Driver/MSVCToolChain.cpp:16
rnk added a comment.
What happens with virtual bases?
struct B { int x; };
struct D : virtual B { int y; };
void test() { D d = {1, .y = 2}; }
Comment at: lib/Sema/SemaInit.cpp:2250
+if (auto *CXXRD = dyn_cast(RT->getDecl()))
+ FieldIndex += CXXRD->getNumBases
rnk accepted this revision.
rnk added a comment.
lgtm
https://reviews.llvm.org/D28705
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D29198
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D29152
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: lib/Driver/MSVCToolChain.cpp:34
+ #if 0
+#define USE_VS_SETUP_CONFIG
+ #endif
hamzasood wrote:
> rnk wrote:
> > What are the outstanding iss
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Repository:
rL LLVM
https://reviews.llvm.org/D29304
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailm
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Can you check if we got this wrong in clang 3.8 and 3.9? I'm wondering if this
was a regression, or if we always got this wrong.
https://reviews.llvm.org/D29350
___
rnk accepted this revision.
rnk added a comment.
Sorry, missed this, looks good.
Repository:
rL LLVM
https://reviews.llvm.org/D28989
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit
rnk added a comment.
This is ready to land. Do you need someone to commit this?
https://reviews.llvm.org/D28365
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL293923: [Driver] Updated for Visual Studio 2017 (authored by
rnk).
Changed prior to commit:
https://reviews.llvm.org/D28365?vs=86502&id=86865#toc
Repository:
rL LLVM
https://reviews.llvm.org/D28365
rnk added a comment.
I had to revert this because it doesn't pass tests on Linux. Can you look into
that and resubmit after fixing those test failures?
Repository:
rL LLVM
https://reviews.llvm.org/D28365
___
cfe-commits mailing list
cfe-commits@
rnk added a comment.
Doesn't your fix mean that the tests will fail on a Windows machine that
doesn't have VS because LLVM was built with mingw? Usually in these situations
we provide some way to provide a fake toolchain.
https://reviews.llvm.org/D28365
_
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D29520
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk added inline comments.
Comment at: test/Misc/inline-asm-diags.c:1
+// RUN: not %clang -c --target=armv7a-arm-none-eabi %s -o /dev/null 2>&1 |
FileCheck %s
+
Use `clang -cc1 -S -o /dev/null -verify` to test this.
https://reviews.llvm.org/D29770
_
rnk added a comment.
I think there's something wrong with clang's implementation of __float128. It
seems fundamentally incompatible with libstdc++'s __is_floating_point_helper
mechanism. We can definitely take this patch to unblock things, but we should
file a bug about investigating this all t
rnk added inline comments.
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1382
+if (const MCConstantExpr *CE =
+dyn_cast_or_null(Val)) {
+ StringRef ErrMsg;
rnk wrote:
> Please use clang-format here and elsewhere
not addressed
rnk added a comment.
Did you locally add a test case for the dllimport member function pointer
template argument?
Comment at: lib/Sema/SemaTemplate.cpp:5704
+ else
+NPV = isNullPointerValueTemplateArgument(S, Param, ParamType, ResultArg);
+
Think we shoul
rnk added a comment.
I thought the intention of -Wcomma was to warn on practically all non-macro
uses of the comma operator. I know it's silly to cast void to void, but I seem
to recall that this was an intentional style-ish warning like -Wparentheses,
which encourages `if ((x = y))` for intent
rnk added a comment.
This appears to affect our behavior under the MS ABI. We still seem to use this
logic from CGVTables, even though that code is primarily Itanium related. In
http://crbug.com/738468 we're seeing the LNK4102 linker warning, which says:
"export of deleting destructor (name); i
rnk added a comment.
Have a minute to get to this? My attempt to work around the bug in Chromium was
incomplete.
https://reviews.llvm.org/D34714
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/
rnk added a comment.
I discussed this with @rsmith and we think the correct fix is to update the
DefinitionData bits computed when adding special members. I haven't reviewed
this patch in detail, but we came to the conclusion that Clang's optimization
to avoid implicit special member declaratio
rnk commandeered this revision.
rnk edited reviewers, added: majnemer; removed: rnk.
rnk added a comment.
@majnemer will be back Monday, grabbing this.
https://reviews.llvm.org/D34714
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://li
This revision was automatically updated to reflect the committed changes.
Closed by commit rL307446: [MS] Don't statically initialize dllimport member
function pointers (authored by rnk).
Changed prior to commit:
https://reviews.llvm.org/D34714?vs=104289&id=105768#toc
Repository:
rL LLVM
ht
rnk added a comment.
In https://reviews.llvm.org/D35259#805284, @erichkeane wrote:
> Oren discovered this miss to the original implementation. I'd reviewed this
> internally quite a bit.
>
> The reason for the Win32ABI test is that MSVC 'long double' is actually small
> enough for SSE register
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
> Some non-windows targets using MS extensions define _WIN32 for compatibility
> with Windows but do not have MSVC compatibility.
So, these hypothetical targets have the Win32 API available, but the
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: lib/CodeGen/TargetInfo.cpp:8463
Kind = AArch64ABIInfo::DarwinPCS;
+else if (Triple.getOS() == llvm::Triple::Win32)
+ Kind = AArch64ABIInfo::Win64;
rnk added inline comments.
Comment at: include/clang/Basic/BuiltinsAArch64.def:65
+// Win64-compatible va_list functions
+BUILTIN(__builtin_ms_va_start, "vc*&.", "nt")
+BUILTIN(__builtin_ms_va_end, "vc*&", "n")
I strongly suspect that Microsoft will never adopt a
rnk added inline comments.
Comment at: include/clang/Basic/BuiltinsAArch64.def:65
+// Win64-compatible va_list functions
+BUILTIN(__builtin_ms_va_start, "vc*&.", "nt")
+BUILTIN(__builtin_ms_va_end, "vc*&", "n")
mstorsjo wrote:
> rnk wrote:
> > I strongly suspect
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Repository:
rL LLVM
https://reviews.llvm.org/D35378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailm
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
Thanks, looks great!
https://reviews.llvm.org/D34475
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
rnk added inline comments.
Comment at: lib/CodeGen/TargetInfo.cpp:3516
+ // calling convention is used.
if (IsRegCall && FI.getReturnType()->getTypePtr()->isRecordType() &&
!FI.getReturnType()->getTypePtr()->isUnionType()) {
This is incorrect. We shoul
rnk requested changes to this revision.
rnk added a comment.
This revision now requires changes to proceed.
This doesn't seem like an acceptable workaround, surely this regresses
functionality for arrays with more than UINT_MAX elements.
https://reviews.llvm.org/D34873
__
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D35546
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk added inline comments.
Comment at: include/clang/Basic/Attr.td:2421
-def SelectAny : InheritableAttr, TargetSpecificAttr {
+def SelectAny : InheritableAttr, TargetSpecificAttr {
let Spellings = [Declspec<"selectany">, GCC<"selectany">];
Prazek wrote:
> d
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Repository:
rL LLVM
https://reviews.llvm.org/D33277
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailm
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D35259
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D35757
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
Repository:
rL LLVM
https://reviews.llvm.org/D35903
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailm
rnk added a comment.
I'm not really sure this is a bug. The point of -Wshadow is to warn on valid
but possibly confusing code. Shadowing a variable is perfectly legal, but
people think it's confusing, so we implemented this warning. Are we concerned
that people might get confused between the di
rnk added inline comments.
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:3867
+// subsequent catches in the same try statement to catch such a rethrow.
+// See C++ DR 388.
+if (!CanBindToTemporary && MaybeTypeMatch) {
Based on our conversation, this fix f
rnk added a comment.
OK. My concern is that users need to use `__attribute__((used))` or something
more robust if they want SVN identifiers to reliably appear in the output.
Adding this flag just creates a trap that will fail once they turn on `-O2`.
I'd rather not have it in the interface to a
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
Looks good!
https://reviews.llvm.org/D42208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-
rnk added a subscriber: aprantl.
rnk added a comment.
@aprantl
Repository:
rC Clang
https://reviews.llvm.org/D42351
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D42455
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
This is actually consistent with what Microsoft does for dllimport classes.
They don't have key functions, but they do import vftables when a class is
dllimport and the constructor is inline. They n
rnk added a comment.
In https://reviews.llvm.org/D42768#994458, @rjmccall wrote:
> No, I mean things like `void foo(__attribute__((swiftcall)) void
> (*fnptr)());`.
Yeah, this was the example I was going to bring up. There's no function
parameter declaration to put the NNS on.
So, here's an
This revision was automatically updated to reflect the committed changes.
Closed by commit rL294978: [CodeGen] Treat auto-generated __dso_handle symbol
as HiddenVisibility (authored by rnk).
Changed prior to commit:
https://reviews.llvm.org/D29843?vs=88068&id=88229#toc
Repository:
rL LLVM
h
rnk added a comment.
I committed a modified patch that set the visibility on the global after
creating it to avoid adding an optional boolean parameter. Thanks for the
report and patch!
Repository:
rL LLVM
https://reviews.llvm.org/D29843
___
cf
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm, thanks!
https://reviews.llvm.org/D29912
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cf
rnk accepted this revision.
rnk added a comment.
lgtm, do you need me to land this? Sorry I took a while to get around to this.
https://reviews.llvm.org/D29464
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mai
rnk requested changes to this revision.
rnk added a comment.
This revision now requires changes to proceed.
Actually, I re-read the concerns about omp.h. We should get that ironed out
first.
https://reviews.llvm.org/D29464
___
cfe-commits mailing l
rnk added a comment.
If I understand correctly, these GCC version specific include directories are
equivalent to clang's resource include directory. We shouldn't have those on
the include search path, so in principle, this seems like the right change.
https://reviews.llvm.org/D29464
___
rnk accepted this revision.
rnk added a comment.
So far as I can tell, Clang on Linux supports none of the headers under
discussion here, and that's been OK for some time. None of the headers in
question appear to actually work today, so I think we should remove them from
the search path.
Rega
rnk added inline comments.
Comment at: test/CodeGenCXX/static-init.cpp:14
+// CHECK98: @_ZZN5test414useStaticLocalEvE3obj = linkonce_odr global
%"struct.test4::HasVTable" zeroinitializer, comdat, align 8
+// CHECK11: @_ZZN5test414useStaticLocalEvE3obj = linkonce_odr global { i8*
rnk added inline comments.
Comment at: docs/LanguageExtensions.rst:2349
+attribute is supported by the pragma by referring to the
+:doc:`individual documentation for that attribute `.
arphaman wrote:
> efriedma wrote:
> > arphaman wrote:
> > > arphaman wrote:
> >
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D29772
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk added a comment.
My understanding is that Windows doesn't allow you to delete a file that has
been opened and mapped into any process, even if it has been opened with
FILE_SHARE_DELETE. One way to work around this would be to avoid memory mapping
files in the SourceManager when using the Re
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
We should file a bug against the Rewriter interface. It should really take
MemoryBuffers from the caller and handle this for them.
https://reviews.llvm.org/D30385
_
rnk added a comment.
This functionality doesn't feel worth a driver flag. It customizes a very small
aspect of a Microsoft extension that was only implemented for compatibility. It
also reimplements the logic that LLVM uses in the backend to put global
variables in .bss or .data. Can you elabor
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
https://reviews.llvm.org/D30518
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
rnk added inline comments.
Comment at: include/clang/Driver/ToolChain.h:305
+ // as OpenMP) to find arch-specific libraries.
+ const std::string getArchSpecificLibPath() const;
+
Why const qualify the std::string?
Comment at: lib/Driver/Tools
rnk created this revision.
Because of the existence branches out of GNU statement expressions, it
is possible that emitting cleanups for a full expression may cause the
new insertion point to not be dominated by the result of the inner
expression. Consider this example:
struct Foo { Foo(); ~Foo
rnk accepted this revision.
rnk added a comment.
Looks good with a minor comment about a comment in the test case.
Comment at: lib/Driver/Tools.cpp:2007-2009
+ // In the cross-compilation case, arch-specific library path is likely
+ // unavailable at runtime.
+ if (TC.isCros
rnk added a comment.
Do you guys think that maybe -flto should imply -fuse-ld=lld, or is that too
much magic?
Repository:
rL LLVM
https://reviews.llvm.org/D30239
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-b
rnk added inline comments.
Comment at: lib/CodeGen/CGExprComplex.cpp:204-206
+Scope.ensureDominatingValue(&Vals.first);
+Scope.ensureDominatingValue(&Vals.second);
+Scope.ForceCleanup();
rsmith wrote:
> I'm a little concerned about the loose connectio
rnk updated this revision to Diff 90743.
rnk added a comment.
- comments
https://reviews.llvm.org/D30590
Files:
lib/CodeGen/CGCleanup.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprComplex.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGenCXX/stmtexpr.cpp
Ind
rnk added a comment.
I think we should really change this code to filter out clobber registers that
do not name valid gcc inline asm clobbers. That's basically what we require
later. I think if you change this condition to just check
Target.isValidGCCRegisterName.
Repository:
rL LLVM
https
1701 - 1800 of 1990 matches
Mail list logo
