hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D34578#801357, @dexonsmith wrote:
> Ping! Hal, you committed r283051. Do you have a problem with this?
It looks like you're just renaming `__libcpp_isnan` and
hfinkel added inline comments.
Comment at: lib/Driver/ToolChains/Cuda.cpp:474
+for (StringRef Opt : OptList) {
+ AddMArchOption(DAL, Opts, Opt);
+}
gtbercea wrote:
> hfinkel wrote:
> > Shouldn't you be adding all of the options, not just the -march=
hfinkel updated this revision to Diff 96264.
hfinkel added a comment.
Output metadata to provide the types of globals (similar to how Clang marks
globals for asan).
https://reviews.llvm.org/D32199
Files:
include/clang/Basic/Sanitizers.def
include/clang/Driver/SanitizerArgs.h
hfinkel added inline comments.
Comment at: lib/Driver/ToolChain.cpp:808
+ continue;
+ } else if (XOpenMPTargetNoTriple)
+// Passing device args: -Xopenmp-target -opt=val.
Please include {} around this else-if code, even though it is not
hfinkel added inline comments.
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3068
+def err_impcast_complex_scalar : Error<
+ "implicit conversion discards imaginary component: %0 to %1">;
def warn_impcast_float_precision : Warning<
I think that,
hfinkel added a comment.
What's going on with the OpenMP test changes?
https://reviews.llvm.org/D35817
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hfinkel added a comment.
In https://reviews.llvm.org/D29654#835392, @gtbercea wrote:
> In https://reviews.llvm.org/D29654#835371, @arphaman wrote:
>
> > The last RUN line in the new commit triggers the same assertion failure:
>
...
> Hi Alex, I'm not sure why it's failing as I can't reproduce
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
In https://reviews.llvm.org/D35817#835740, @t.p.northover wrote:
> > What's going on with the OpenMP test changes?
>
> Compound assignment operators like "real /= complex" become illegal
hfinkel added a comment.
In https://reviews.llvm.org/D34158#837130, @joerg wrote:
> In https://reviews.llvm.org/D34158#836026, @jyknight wrote:
>
> > In https://reviews.llvm.org/D34158#827178, @joerg wrote:
> >
> > > (2) It adds magic behavior that can make debugging more difficult.
> > >
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
LGTM. Thanks for all of your work on this!
Comment at: test/Driver/openmp-offload.c:603
+
+/// Check -Xopenmp-target=powerpc64le-ibm-linux-gnu -march=pwr8 is passed when
hfinkel added inline comments.
Comment at: include/clang/AST/Expr.h:3814
+/// BuiltinSourceLocExpr - Represents a function call to one of
+/// __builtin_LINE(), __builtin_COLUMN(), __builtin_FUNCTION(), or
BuiltinSourceLocExpr -> SourceLocExpr
hfinkel added inline comments.
Comment at: lib/Driver/ToolChains/Cuda.cpp:388
+
+ // add paths specified in LIBRARY_PATH environment variable as -L options.
+ addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH");
Comment sentences should start with a capital
hfinkel added a comment.
I prefer `__has_attribute_feature` to enhancement. I don't see why we need a
new macro for this, however. Why not just use
`__has_feature(overloadable_unmarked)` or similar?
https://reviews.llvm.org/D33904
___
cfe-commits
hfinkel added a comment.
In https://reviews.llvm.org/D29651#720179, @tstellar wrote:
> Why is this necessary?
To put it another way, what is special about LIBRARY_PATH in this regard? Do we
already pass paths specified with `-L`? I'm also slightly concerned here with
the custom parsing code.
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
LGTM
Repository:
rL LLVM
https://reviews.llvm.org/D29654
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
hfinkel added a comment.
In general, this patch seems to be missing tests (unless it is actually NFC, or
you can't write tests yet, which, in either case, need to be explained).
Comment at: lib/Driver/ToolChains/Cuda.cpp:217
+ std::vector GPUArchNames;
+ // If this is an
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D34357
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
hfinkel added a comment.
In https://reviews.llvm.org/D37436#869467, @aaron.ballman wrote:
> In https://reviews.llvm.org/D37436#869462, @hfinkel wrote:
>
> > I think that I misunderstood your concern. Let me see if I can summarize
> > your position: You believe that, when GCC implements this
hfinkel added inline comments.
Comment at: test/Driver/openmp-offload-gpu.c:140
+
+/// Check that error is not thrown by toolchain when no cuda lib device is
found when using -c -S.
+/// Check that the flag is passed when -fopenmp-relocatable-target is used.
Do
hfinkel added a comment.
In https://reviews.llvm.org/D38113#877874, @Anastasia wrote:
> The problem of adding this attribute conservatively for all functions is that
> it prevents some optimizations to happen. I agree to commit this as a
> temporary fix to guarantee correctness of generated
hfinkel added a comment.
Please add a test case.
Repository:
rL LLVM
https://reviews.llvm.org/D37913
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hfinkel added inline comments.
Comment at: lib/Driver/ToolChains/Cuda.cpp:442
+SmallString<256> Name = llvm::sys::path::relative_path(II.getFilename());
+SmallString<256> FullPath = llvm::sys::path::root_path(II.getFilename());
hfinkel added a comment.
Some suggested improvements to the English in the documentation...
Comment at: docs/UsersManual.rst:700
+
+Configuration files group command line options and allow to specify all of
+them just by referencing the configuration file. They may be used,
hfinkel added inline comments.
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:335
+def warn_drv_fine_grained_bitfield_accesses_ignored : Warning<
+ "option '-ffine-grained-bitfield-accesses' cannot be enabled together with
sanitizer; flag ignored">,
+ InGroup;
hfinkel added a comment.
Is this still being worked on?
https://reviews.llvm.org/D31417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hfinkel added a comment.
In https://reviews.llvm.org/D37624#885438, @choikwa wrote:
> > Can you get more information on what GCC actually implemented and why? It's
> > not clear to me that ignoring the namespaces is the most-useful way to do
> > this. I don't want to emulate GCC bugs, but
hfinkel updated this revision to Diff 119103.
hfinkel added a comment.
Rebased.
https://reviews.llvm.org/D32199
Files:
include/clang/Basic/Sanitizers.def
include/clang/Driver/SanitizerArgs.h
lib/CodeGen/BackendUtil.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGDeclCXX.cpp
hfinkel added inline comments.
Comment at: lib/CodeGen/CGRecordLayoutBuilder.cpp:411
+ // component so as it can be accessed directly with lower cost.
+ auto betterBeSingleFieldRun = [&](RecordDecl::field_iterator Field) {
+if
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: include/clang/Frontend/CodeGenOptions.def:182
CODEGENOPT(SoftFloat , 1, 0) ///< -soft-float.
+CODEGENOPT(FineGrainedBitfieldAccesses, 1, 0) ///<
hfinkel added a comment.
In https://reviews.llvm.org/D37436#868333, @aaron.ballman wrote:
> In https://reviews.llvm.org/D37436#868295, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D37436#867965, @aaron.ballman wrote:
> >
> > > In https://reviews.llvm.org/D37436#867287, @rsmith wrote:
> > >
hfinkel added a comment.
Also, please post a full-context patch.
https://reviews.llvm.org/D37436
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hfinkel added a comment.
In https://reviews.llvm.org/D37436#869445, @aaron.ballman wrote:
> In https://reviews.llvm.org/D37436#869350, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D37436#868333, @aaron.ballman wrote:
> >
> > > In https://reviews.llvm.org/D37436#868295, @hfinkel wrote:
> > >
hfinkel added a comment.
In https://reviews.llvm.org/D37436#867965, @aaron.ballman wrote:
> In https://reviews.llvm.org/D37436#867287, @rsmith wrote:
>
> > If this is just supposed to be an experiment to get feedback on the
> > feature, then I don't think we should be treating it as a
hfinkel added a comment.
You seem to be only changing the behavior for the "separatable" fields, but I
suspect you want to change the behavior for the others too. The bitfield would
be decomposed into shards, separated by the naturally-sized-and-aligned fields.
Each access only loads its
hfinkel added inline comments.
Comment at: ReleaseNotes.rst:82
+- ``-Wnull-pointer-arithmetic`` now warns about performing pointer arithmetic
+ on a null pointer. It has an undefined behavior if the offset is nonzero.
+
We can probably just make this one entry:
hfinkel added a comment.
Please also add a C++ test to check the mangling-related features.
Comment at: lib/CodeGen/CodeGenFunction.cpp:419
+// Assume that __cxa_demangle is provided by libcxxabi (except for Windows).
+extern "C" char *__cxa_demangle(const char *mangled_name,
hfinkel added inline comments.
Comment at: test/CodeGenCXX/instrument-functions.cpp:8
+// RUN: %clang -S -emit-llvm -o - %s -finstrument-functions
-finstrument-functions-exclude-function-list=test3 | FileCheck %s
--check-prefix=NOFUNC
+// RUN: %clang -S -emit-llvm -o - %s
hfinkel added a comment.
In https://reviews.llvm.org/D37624#885288, @choikwa wrote:
> - add comment to CPP test to explain usage
Thanks. Please also add some tests showing matching overloaded functions,
functions with template parameters, etc.
Do we need to strip whitespace before trying to
hfinkel added a comment.
In https://reviews.llvm.org/D37624#885295, @choikwa wrote:
> In https://reviews.llvm.org/D37624#885290, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D37624#885288, @choikwa wrote:
> >
> > > - add comment to CPP test to explain usage
> >
> >
> > Thanks. Please also
hfinkel added a comment.
In https://reviews.llvm.org/D37624#885311, @choikwa wrote:
> In https://reviews.llvm.org/D37624#885308, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D37624#885295, @choikwa wrote:
> >
> > > In https://reviews.llvm.org/D37624#885290, @hfinkel wrote:
> > >
> > > > In
hfinkel added a comment.
In https://reviews.llvm.org/D31417#886826, @huntergr wrote:
> In https://reviews.llvm.org/D31417#886441, @hfinkel wrote:
>
> > Is this still being worked on?
>
>
> Hi, yes it is. Sorry for the delay in posting new changes but priorities
> shifted a bit and I had to work
hfinkel added a comment.
I'd really like to see this done in some way such that we can issue a warning
along with generating well-defined code. The warning can specifically state
that the code is using an extension, etc.
https://reviews.llvm.org/D37042
hfinkel added a comment.
In https://reviews.llvm.org/D37042#852733, @efriedma wrote:
> It would be nice to warn on any nullptr arithmetic, yes. (We probably want
> the wording of the warning to be a bit different if we're activating this
> workaround.)
+1 (I'll likely want the ability to
hfinkel added a comment.
Can you explain briefly what is required of the system in order to support this
(is it just ifunc support in the dynamic linker / loader)?
In general, there are a number of places in this patch, at the Sema layer
(including in the error messages), that talk about how
hfinkel added a comment.
In https://reviews.llvm.org/D38596#889697, @erichkeane wrote:
> In https://reviews.llvm.org/D38596#889682, @hfinkel wrote:
>
> > Can you explain briefly what is required of the system in order to support
> > this (is it just ifunc support in the dynamic linker /
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D38372
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
hfinkel updated this revision to Diff 117619.
hfinkel added a comment.
Herald added a subscriber: kosarev.
Rebased.
https://reviews.llvm.org/D32199
Files:
include/clang/Basic/Sanitizers.def
include/clang/Driver/SanitizerArgs.h
lib/CodeGen/BackendUtil.cpp
lib/CodeGen/CGDecl.cpp
hfinkel added a comment.
In https://reviews.llvm.org/D38113#882187, @Anastasia wrote:
> In https://reviews.llvm.org/D38113#878840, @jlebar wrote:
>
> >
>
...
> Yes, I see this sounds more reasonable indeed. Btw, currently LLVM can remove
> `convergent` in some cases to recover the
hfinkel added a comment.
> With this patch we avoid unaligned loads and stores, at least on MIPS
> architecture.
Can you please explain the nature of the problematic situations?
Also, this patch does not update the comments that describe the splitting
algorithm. That should be improved.
If
hfinkel added a comment.
In https://reviews.llvm.org/D39005#900973, @jlebar wrote:
> > The first question that comes to mind is what is the link between data
> > layout and name mangling conventions?
>
> I pulled up http://llvm.org/doxygen/classllvm_1_1DataLayout.html and searched
> for
hfinkel added a comment.
In https://reviews.llvm.org/D39160#902918, @spatel wrote:
> Oops - I wrongly made llvm-commits a subscriber rather than cfe-commits. Let
> me know if I should reopen under a new thread to get the patch to hit the
> right mailing list.
Yes, please reopen.
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
I think this looks fine (and I agree that the underlying premise makes sense).
Before you commit, could you also please add some tests with nested union
members?
Repository:
rL LLVM
hfinkel added a comment.
I think this is a good idea.
Comment at: include/clang/Driver/CC1Options.td:407
def verify : Flag<["-"], "verify">,
- HelpText<"Verify diagnostic output using comment directives">;
+ HelpText<"Similar to -verify=expected">;
def
This revision was automatically updated to reflect the committed changes.
Closed by commit rC320908: [VerifyDiagnosticConsumer] support
-verify=prefixes (authored by hfinkel, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D39694
Files:
This revision was automatically updated to reflect the committed changes.
Closed by commit rC320904: [TextDiagnosticBuffer] Fix diagnostic note emission
order (authored by hfinkel, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D40995
Files:
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:937
if (!CapturedVars.empty()) {
+ // There's somehting to share, add the attribute
+
hfinkel added inline comments.
Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:398
+// DToken is foo-bar-warning, but foo is the only -verify prefix).
+if (Prefixes.end() == std::find(Prefixes.begin(), Prefixes.end(), DToken))
+ continue;
>
hfinkel added a comment.
In https://reviews.llvm.org/D40594#940571, @spatel wrote:
> Added a blurb to the LangRef with https://reviews.llvm.org/rL319437, so I
> think we can abandon this patch.
Sounds good.
https://reviews.llvm.org/D40594
___
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D39694
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
hfinkel added inline comments.
Comment at: lib/CodeGen/CGExprComplex.cpp:773
// supported imaginary types in addition to complex types.
-if (RHSi) {
+if (RHSi && !FMF.isFast()) {
BinOpInfo LibCallOp = Op;
fhahn wrote:
> Would the following
hfinkel added inline comments.
Comment at: lib/Driver/Driver.cpp:641
+
+static std::string getAbsolutePath(StringRef P) {
+ if (P.empty())
Use llvm::sys::fs::make_absolute instead of this.
https://reviews.llvm.org/D24933
hfinkel added inline comments.
Comment at: test/CodeGen/tbaa-array.cpp:24
+// CHECK-DAG: [[TAG_A_i]] = !{[[TYPE_A:!.*]], [[TYPE_int:!.*]], i64 0, i64 4}
+// CHECK-DAG: [[TAG_C_i]] = !{[[TYPE_C:!.*]], [[TYPE_int:!.*]], i64 0, i64 16}
+// CHECK-DAG: [[TYPE_A]] =
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D40299
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
hfinkel added a comment.
In https://reviews.llvm.org/D41394#959715, @rjmccall wrote:
> Rewriting some of the most basic tests would be fine. Please either use new
> FileCheck lines or clone the existing tests, since we don't really know how
> long this transition will last.
+1
Otherwise,
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
LGTM
https://reviews.llvm.org/D39812
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
hfinkel added a comment.
In https://reviews.llvm.org/D39611#921923, @spatel wrote:
> Thanks for the clarification!
>
> If I'm reading this properly, we should make the same kind of change as in
> https://reviews.llvm.org/D39481 ('c' -> 'e') for most of complex.h. Ie, the
> standard allows
hfinkel added a comment.
> When Sema sees this code during compilation, it can not tell whether there is
> an error. Calling foo from the host code is perfectly valid. Calling it from
> device code is not. CUDADiagIfDeviceCode creates 'postponed' diagnostics
> which only gets emitted if we
hfinkel added a comment.
In https://reviews.llvm.org/D40144#927828, @tcanens wrote:
> At least for GCC, it should use `__builtin_launder`.
I presume we'll need to add something similar for Clang as well.
> Also needs to implement "The program is ill-formed if `T` is a function type
> or
hfinkel added a comment.
In https://reviews.llvm.org/D39481#914174, @craig.topper wrote:
> There's an oddity with fma. The version without __builtin has 'e' already
Something that is potentially relevant: POSIX says that fma() can set errno
hfinkel added a comment.
In https://reviews.llvm.org/D39611#918275, @spatel wrote:
> Patch updated:
> I don't know if we have agreement on the behavior that we want yet,
I just sent a note to the Austin group mailing list to see if the POSIX folks
agree with my reading. I'll follow up.
>
hfinkel added a comment.
In https://reviews.llvm.org/D39611#918654, @hfinkel wrote:
> In https://reviews.llvm.org/D39611#918275, @spatel wrote:
>
> > Patch updated:
> > I don't know if we have agreement on the behavior that we want yet,
>
>
> I just sent a note to the Austin group mailing list
hfinkel added a comment.
In https://reviews.llvm.org/D39611#915799, @efriedma wrote:
> > In general, because all of these functions are well defined over the entire
> > complex domain, they don't have errors. The "no errors defined" is likely
> > correct.
>
> One, it is not true that all these
hfinkel added a comment.
In https://reviews.llvm.org/D39611#915560, @efriedma wrote:
> > The POSIX docs all have language like this for complex calls:
> > "No errors are defined."
>
> I would not trust the POSIX documentation.
>
> carg() is an alias for atan2(), and cabs() is an alias for
hfinkel added inline comments.
Comment at: lib/Frontend/CompilerInvocation.cpp:1095
+ if (Diags) {
+Diags->Report(diag::err_drv_invalid_value) << "-verify=" << Prefix;
+Diags->Report(diag::note_drv_verify_prefix_unique);
jdenny wrote:
>
hfinkel added a comment.
In https://reviews.llvm.org/D39455#943182, @aheejin wrote:
> Once you confirm the bug, could you possibly revert the patch?
I agree. We should revert this. The relevant part of the test case is:
short *q;
p->u.vec[i] = 0;
q = >u.vec[16];
*q = 1;
return
hfinkel added inline comments.
Comment at: lib/Driver/Driver.cpp:630
+ for (const char *Dir : Directories) {
+assert(Dir);
+FilePath.clear();
assert(Dir && "Directory path should not be null");
Comment at: lib/Driver/Driver.cpp:637
hfinkel added inline comments.
Comment at: lib/Driver/Driver.cpp:661
+
+ // If not found, try searching the directory where executable resides.
+ FilePath.clear();
hfinkel wrote:
> Why not just add this directory to the end of the list of directories?
(by
hfinkel added a comment.
In https://reviews.llvm.org/D39138#904747, @rjmccall wrote:
> Okay, if this is just for your own checking, I'd rather not take it. It's
> not a significant compile-time cost, but there's no reason to pay it at all.
In that case, can we take it? I'd rather have
hfinkel added a comment.
> what is breaking the ELF interposition rules
Frankly, in retrospect, ELF's interposition rules (or at least the defaults for
those rules), seem suboptimal on several fronts. While breaking them has some
unfortunate consistency implications, I don't consider breaking
hfinkel added subscribers: vit9696, hfinkel.
hfinkel added a comment.
Noting that, as @vit9696 pointed out in https://reviews.llvm.org/D38554, this
does not suppress uses of the PLT that occur from backend/optimizer-generated
functions (e.g., calls into compiler-rt and similar).
hfinkel added a comment.
In https://reviews.llvm.org/D39138#905445, @rjmccall wrote:
> In https://reviews.llvm.org/D39138#905184, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D39138#904747, @rjmccall wrote:
> >
> > > Okay, if this is just for your own checking, I'd rather not take it.
> >
hfinkel added a comment.
Makes sense to me.
Repository:
rC Clang
https://reviews.llvm.org/D46441
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hfinkel added a comment.
In https://reviews.llvm.org/D39053#1093977, @asb wrote:
> Just wanted to explicitly say that I'm happy the updated patch reflects the
> changes to docs and comments I requested. @hfinkel - are you happy for this
> to land now?
Yes, go ahead.
Some of these benefits
hfinkel added a comment.
> I have some doubts whether this will ever be used sensibly in the real world,
> but can
> be useful for testing and was not difficult to put together.
I definitely support having pragmas for these kinds of loop transformations. In
my experience, they are used.
hfinkel added a comment.
In https://reviews.llvm.org/D47849#1124638, @Hahnfeld wrote:
> IMO this goes into the right direction, we should use the fast implementation
> in libdevice. If LLVM doesn't lower these calls in the NVPTX backend, I think
> it's ok to use header wrappers as CUDA already
hfinkel added a comment.
In https://reviews.llvm.org/D47267#1123038, @Meinersbur wrote:
> In https://reviews.llvm.org/D47267#1123013, @dmgreen wrote:
>
> > I see your point about the mix of underscores. "nounroll_and_jam" also
> > comes from the Intel docs, and theres already "nounroll" vs
hfinkel added inline comments.
Comment at: test/Frontend/compiler-options-dump.cpp:3
+// RUN: %clang_cc1 -compiler-options-dump -std=c++17 %s -o - | FileCheck %s
--check-prefix=CXX17
+// RUN: %clang_cc1 -compiler-options-dump -std=c99 -ffast-math -x c %s -o - |
FileCheck %s
hfinkel added a comment.
In https://reviews.llvm.org/D48808#1149828, @Meinersbur wrote:
> In https://reviews.llvm.org/D48808#1149549, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D48808#1149534, @ABataev wrote:
> >
> > >
> >
> >
> > Michael, can you please add a test with two inner loops,
hfinkel added a comment.
In https://reviews.llvm.org/D48721#1150333, @bjope wrote:
> I tried running
>
> /clang -cc1 -O3 -funroll-loops -S -emit-llvm pragma-do-while-unroll.cpp -o
> - -mllvm -print-after-all
>
>
>
> and I get this
>
> ...
> !2 = distinct !{!2, !3}
> !3 =
hfinkel added a comment.
In https://reviews.llvm.org/D48808#1149862, @Meinersbur wrote:
> In https://reviews.llvm.org/D48808#1149845, @hfinkel wrote:
>
> > We specifically defined the metadata to support nested loops. The LangRef
> > says, "The llvm.mem.parallel_loop_access metadata refers to a
hfinkel added a comment.
In https://reviews.llvm.org/D48808#1149534, @ABataev wrote:
> I don't think that this is the intended behavior of the `#pragma clang loop`.
> it is better to ask the author of this pragma is this correct or not.
It is the intended behavior that the memory accesses are
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.
LGTM
Repository:
rL LLVM
https://reviews.llvm.org/D39083
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
hfinkel added inline comments.
Comment at: lib/Driver/Driver.cpp:706
+
+ // Determine architecture part of the file name, if it presents.
+ size_t ArchPrefixLen = 0;
if it presents. -> if it is present.
Comment at: lib/Driver/Driver.cpp:739
hfinkel added a comment.
Quick question: We're talking about a few different variants on this feature
now: Post-inlining instrumentation, only marking function entries, and not
passing the function address. Do you intend to use all of these things
separately? I ask because, taken together,
hfinkel added a comment.
In https://reviews.llvm.org/D39053#906513, @spetrovic wrote:
> Well, basically I'm just expanding the existing algorithm, why should we
> split fields just in case when current field is integer,
> I'm not resolving specific problem with unaligned loads/stores on MIPS.
hfinkel added a comment.
In https://reviews.llvm.org/D39204#905860, @efriedma wrote:
> I think you're understanding the semantics correctly.
>
> For r265521, look again at the implementation of
> llvm::checkUnaryFloatSignature; if "I.onlyReadsMemory()" is true, we somehow
> proved the call
hfinkel added a comment.
Please upload diffs with full context.
Comment at: include/clang/Driver/Options.td:1907
def mvsx : Flag<["-"], "mvsx">, Group;
+def msave_toc_indirect : Flag<["-"], "msave-toc-indirect">,
Group;
def mno_vsx : Flag<["-"], "mno-vsx">, Group;
hfinkel added a comment.
In https://reviews.llvm.org/D39457#961824, @Hahnfeld wrote:
> @hfinkel I think you requested this documentation on the mailing list. Can
> you take a look if it matches your expectations so we can get this bundled in
> the 6.0 release?
Yes, this looks good. Thank
hfinkel added a comment.
In https://reviews.llvm.org/D42366#1012211, @rjmccall wrote:
> I'm fine with that plan. LGTM.
Thanks.
We should probably discuss what we want to do with unknown-sized things.
UINT64_MAX is the conservative answer, but maybe there's some value in just
having 0 mean
hfinkel added a comment.
In https://reviews.llvm.org/D42366#1014546, @rjmccall wrote:
> In https://reviews.llvm.org/D42366#1014157, @kosarev wrote:
>
> > I think zero would serve better as the unknown-size value. People who are
> > not aware of TBAA internals would guess that since zero-sized
101 - 200 of 327 matches
Mail list logo
