- User Since
- Mar 27 2015, 11:23 AM (212 w, 5 d)
Fri, Apr 19
There shouldn't be an empty string ("") in the asm output -- that should be a reference to the "l_yes" label, not the empty string. That seems very weird...
Tue, Apr 9
Mon, Apr 8
If you change to condition on GLIBC instead of linux, I think this is fine to commit (although it still seems unfortunate to me that we're carrying an abandoned codebase which depends on weird broken stuff like this...)
Looks reasonable to me.
Can you please write a test case for this? There's some existing sparcv9 spill tests in llvm/test/CodeGen/SPARC/64spill.ll.
Sat, Apr 6
Fri, Apr 5
This commit message is rather bare-bones -- I hope that the ultimate commit message will be updated to contain more than just a reference to the RFC thread.
Thu, Apr 4
This patch is not correct -- the crash is not with varargs functions specifically, llvm also crashes for a function declared to explicitly take a float/double. The Win64 calling convention code assigns values to xmm registers, but it should not when sse is disabled.
Wed, Apr 3
After looking at this a bit more, I realized that this is still insufficient. There's *many* more target-specific constraints than just x86 "i" which sign extend constant integer arguments -- and we'll need to apply this change to every one of them, in all targets.
This looks like the same bug is present in the generic code in llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp as well.
I would question whether it's actually worth it to attempt to keep software working that seems to have been abandoned for a decade as part of the llvm test suite? Should this code just be removed, instead of patched?
Mon, Apr 1
Sounds like this file is missing #include <math.h> (it probably only #include <cmath>). If it wants to use ::log10, it should #include <math.h>. If it wants to use <cmath>, it should use std::log10.
Fri, Mar 29
It'd be nice if we knew why adding new nodes to the worklist seemed to cause an issue, but the reduced form as here seems okay as far as it goes.
Looks fine again.
Thu, Mar 28
Wed, Mar 27
Tue, Mar 26
Not sure if the PPC issue was a different one, but the commit that reverted this was:
Mar 25 2019
Mar 22 2019
Mar 20 2019
Mar 18 2019
Could you add a comment to the CL description noting that a bunch of the calls to AddToWorklist in DAGCombiner.cpp that follow a DAG.getNode() are now redundant, and should be cleaned up in the future.
Sorry, forgot to re-ping this in a timely manner. :)
Mar 14 2019
It looks like this was generated because a pull request against the upstream repo was created containing this commit.
Mar 6 2019
Ah -- I now understand your concern, and managing user expectations appropriately does seem like a potential concern.
Mar 5 2019
Is there any reason this needs to be a template -- can't these just be changed to function overloads, instead of template specializations?
The errors disabled by this feature are default-error warnings -- you can already get the same effect by using -Wno-<lots-of-things>. Why is it bad to additionally allow -fpermissive to disable them? (If we have any diagnostics which are currently default-on-warnings which should not _ever_ be disable-able, then maybe we should just fix those?)
Mar 4 2019
Hm -- in GCC, -fpermissive has nothing at all to do with -pedantic/-pedantic-errors, but I suppose it should be fine to do this way.
Mar 2 2019
The intricate initialization-order workarounds apparently don't work in all build modes, so I've updated this code to have constexpr functions and initializations in r355278.
Feb 27 2019
Feb 26 2019
Probably worth adding a note to the release notes, something like "The optimizer will now convert calls to memcmp into a calls to bcmp in some circumstances. Users who are building freestanding code (not depending on the platform's libc) without specifying -ffreestanding may need to either pass -fno-builtin-bcmp, or provide a bcmp function."
Feb 22 2019
Minor tweaks per comments.
Add some wording to LangRef and clang-format.
Feb 20 2019
Feb 19 2019
Feb 16 2019
I still don't like it...It's different, unusual, and IMO surprising to have such a wildcard ignore.
Feb 15 2019
Feb 13 2019
I think this warning (-Wbuiltin-requires-header) doesn't really make sense as its own warning.
OK, I'm happy with all of that.
Looks reasonable to me.
Feb 11 2019
It'd be great to see this somewhat more widely publicized, outside of just the clang community. If libc implementors are aware of the gains and are willing to provide an actually-faster bcmp implementation, it'd be a lot better, than having this optimization that doesn't really optimize anything without users providing their own bcmp implementation.