Ilya Biryukov [Fri, 1 Feb 2019 11:20:13 +0000 (11:20 +0000)]
Disable tidy checks with too many hits
Summary:
Some tidy checks have too many hits in the codebase, making it hard to spot
other results from clang-tidy, therefore rendering the tool less useful.
Two checks were disabled:
- misc-non-private-member-variable-in-classes in the whole LLVM monorepo,
it is very common to have those in LLVM and the style guide does not forbid
them.
- readability-identifier-naming in the clang subtree. There are thousands of
violations in 'Sema.h' alone.
Before the change, 'Sema.h' had >1000 tidy warnings, after the change the number
dropped to 3 warnings (unterminated namespace comments).
Roman Lebedev [Fri, 1 Feb 2019 11:15:13 +0000 (11:15 +0000)]
[X86][BdVer2] Transfer delays from the integer to the floating point unit.
Summary:
I'm unable to find this number in the "AMD SOG for family 15h".
llvm-exegesis measures the latencies of these instructions as `2`,
which matches the latencies specified in "AMD SOG for family 15h".
However if we look at Agner, Microarchitecture, "AMD Bulldozer, Piledriver,
Steamroller and Excavator pipeline", "Data delay between different execution
domains", the int->ivec transfer is listed as `8`..`10`cy of additional latency.
Also, Agner's "Instruction tables", for Piledriver, lists their latencies as `12`,
which is consistent with `2cy` from exegesis / AMD SOG + `10cy` transfer delay.
Additional data point comes from the fact that Agner's "Instruction tables",
for Jaguar, lists their latencies as `8`; and "AMD SOG for family 16h" does
state the `+6cy` int->ivec delay, which is consistent with instr latency of `1` or `2`.
Yevgeny Rouban [Fri, 1 Feb 2019 10:44:43 +0000 (10:44 +0000)]
Provide reason messages for unviable inlining
InlineCost's isInlineViable() is changed to return InlineResult
instead of bool. This provides messages for failure reasons and
allows to get more specific messages for cases where callsites
are not viable for inlining.
James Henderson [Fri, 1 Feb 2019 10:24:55 +0000 (10:24 +0000)]
[llvm-symbolizer][test] Rename and tweak tests using llvm-symbolizer
Prior to this change, there are a few tests called llvm-symbolizer* in
the DebugInfo test area. These really were testing either the DebugInfo
or Symbolizer library, rather than the llvm-symbolizer tool itself, so
this patch renames them to be clearer that they aren't explicitly tests
for llvm-symbolizer (such tests belong in test/tools/llvm-symbolizer).
This patch also reinstates the copying of a DWO file, removed previously
in r352752. The test needs this so that it could possibly fail.
Finally, some of the tests have been simplified slightly by removing
unnecessary switches and/or unused check-prefixes.
James Henderson [Fri, 1 Feb 2019 10:02:42 +0000 (10:02 +0000)]
[doc]Update String Error documentation in Programmer Manual
A while back, createStringError was added to provide easier construction
of StringError instances, especially with formatting options. Prior to
this patch, that the documentation only mentions the standard method of
using it. Since createStringError is slightly shorter to type, and also
provides the formatting options, this patch updates the Programmer's
Manual to use the new function in its examples, and to mention the
printf formatting options. It also fixes a small typo in one of the
examples and removes the unnecessary make_error_code call.
Oliver Stannard [Fri, 1 Feb 2019 09:23:51 +0000 (09:23 +0000)]
[CodeGen] Don't scavenge non-saved regs in exception throwing functions
Previously, LiveRegUnits was assuming that if a block has no successors
and does not return, then no registers are live at the end of it
(because the end of the block is unreachable). This was causing the
register scavenger to use callee-saved registers to materialise stack
frame addresses without saving them in the prologue. This would normally
be fine, because the end of the block is unreachable, but this is not
legal if the block ends by throwing a C++ exception. If this happens,
the scratch register will be modified, but its previous value won't be
preserved, so it doesn't get restored by the exception unwinder.
Alex Bradbury [Fri, 1 Feb 2019 03:53:30 +0000 (03:53 +0000)]
[RISCV] Implement RV64D codegen
This patch:
* Adds necessary RV64D codegen patterns
* Modifies CC_RISCV so it will properly handle f64 types (with soft float ABI)
Note that in general there is no reason to try to select fcvt.w[u].d rather than fcvt.l[u].d for i32 conversions because fptosi/fptoui produce poison if the input won't fit into the target type.
Alex Bradbury [Fri, 1 Feb 2019 03:46:28 +0000 (03:46 +0000)]
[SelectionDAG] Support promotion of the FPOWI integer operand
For targets where i32 is not a legal type (e.g. 64-bit RISC-V),
LegalizeIntegerTypes must promote the integer operand of ISD::FPOWI. As this
is a signed value, this should be sign-extended.
This patch enables all tests in test/CodeGen/RISCVfloat-intrinsics.ll for
RV64, as prior to this patch that file couldn't be compiled for RV64 due to an
assertion when performing codegen for fpowi.
James Y Knight [Fri, 1 Feb 2019 02:28:03 +0000 (02:28 +0000)]
[opaque pointer types] Add a FunctionCallee wrapper type, and use it.
Recommit r352791 after tweaking DerivedTypes.h slightly, so that gcc
doesn't choke on it, hopefully.
Original Message:
The FunctionCallee type is effectively a {FunctionType*,Value*} pair,
and is a useful convenience to enable code to continue passing the
result of getOrInsertFunction() through to EmitCall, even once pointer
types lose their pointee-type.
Then:
- update the CallInst/InvokeInst instruction creation functions to
take a Callee,
- modify getOrInsertFunction to return FunctionCallee, and
- update all callers appropriately.
One area of particular note is the change to the sanitizer
code. Previously, they had been casting the result of
`getOrInsertFunction` to a `Function*` via
`checkSanitizerInterfaceFunction`, and storing that. That would report
an error if someone had already inserted a function declaraction with
a mismatching signature.
However, in general, LLVM allows for such mismatches, as
`getOrInsertFunction` will automatically insert a bitcast if
needed. As part of this cleanup, cause the sanitizer code to do the
same. (It will call its functions using the expected signature,
however they may have been declared.)
Finally, in a small number of locations, callers of
`getOrInsertFunction` actually were expecting/requiring that a brand
new function was being created. In such cases, I've switched them to
Function::Create instead.
[sanitizer-coverage] prune trace-cmp instrumentation for CMP isntructions that feed into the backedge branch. Instrumenting these CMP instructions is almost always useless (and harmful) for fuzzing
JF Bastien [Thu, 31 Jan 2019 23:29:39 +0000 (23:29 +0000)]
Revert "Bump minimum toolchain version"
A handful of bots are still breaking, either because I missed them in my audit,
they were offline, or something else. I'm contacting their authors, but I'll
revert for now and re-commit later.
Thomas Lively [Thu, 31 Jan 2019 23:22:39 +0000 (23:22 +0000)]
[WebAssembly] Fix a regression selecting negative build_vector lanes
Summary:
The custom lowering introduced in rL352592 creates build_vector nodes
with negative i32 operands, but these operands did not meet the value
range constraints necessary to match build_vector nodes. This CL fixes
the issue by removing the unnecessary constraints.
Alex Bradbury [Thu, 31 Jan 2019 22:48:38 +0000 (22:48 +0000)]
[RISCV] Add RV64F codegen support
This requires a little extra work due tothe fact i32 is not a legal type. When
call lowering happens post-legalisation (e.g. when an intrinsic was inserted
during legalisation). A bitcast from f32 to i32 can't be introduced. This is
similar to the challenges with RV32D. To handle this, we introduce
target-specific DAG nodes that perform bitcast+anyext for f32->i64 and
trunc+bitcast for i64->f32.
And it triggers CombineZExtLogicopShiftLoad again, causes a dead loop.
Both forms should generate same instructions, CombineZExtLogicopShiftLoad generated IR looks cleaner. But it looks more difficult to prevent visitANDLike to do the transform, so I prevent CombineZExtLogicopShiftLoad to do the transform if the ZExt is free.
James Y Knight [Thu, 31 Jan 2019 20:35:56 +0000 (20:35 +0000)]
[opaque pointer types] Add a FunctionCallee wrapper type, and use it.
The FunctionCallee type is effectively a {FunctionType*,Value*} pair,
and is a useful convenience to enable code to continue passing the
result of getOrInsertFunction() through to EmitCall, even once pointer
types lose their pointee-type.
Then:
- update the CallInst/InvokeInst instruction creation functions to
take a Callee,
- modify getOrInsertFunction to return FunctionCallee, and
- update all callers appropriately.
One area of particular note is the change to the sanitizer
code. Previously, they had been casting the result of
`getOrInsertFunction` to a `Function*` via
`checkSanitizerInterfaceFunction`, and storing that. That would report
an error if someone had already inserted a function declaraction with
a mismatching signature.
However, in general, LLVM allows for such mismatches, as
`getOrInsertFunction` will automatically insert a bitcast if
needed. As part of this cleanup, cause the sanitizer code to do the
same. (It will call its functions using the expected signature,
however they may have been declared.)
Finally, in a small number of locations, callers of
`getOrInsertFunction` actually were expecting/requiring that a brand
new function was being created. In such cases, I've switched them to
Function::Create instead.
Shoaib Meenai [Thu, 31 Jan 2019 20:32:45 +0000 (20:32 +0000)]
[cmake] Note future cleanup in comment. NFC
CMake 3.6 introduced CMAKE_TRY_COMPILE_PLATFORM_VARIABLES, which solves
precisely the problem that necessitated init_user_prop, so we can switch
over whenever we bump our minimum CMake requirement.
Alina Sbirlea [Thu, 31 Jan 2019 20:13:47 +0000 (20:13 +0000)]
[MemorySSA] Extend removeMemoryAccess API to optimize MemoryPhis.
Summary:
EarlyCSE needs to optimize MemoryPhis after an access is removed and has
special handling for it. This should be handled by MemorySSA instead.
The default remains that MemoryPhis are *not* optimized after an access
is removed.
With this patch the output more closely resembles GNU nm:
``` 00000000 N .debug_abbrev 00000000 n .note.GNU-stack
...
```
This patch calls `getSectionName` for sections that belong to symbols of type `ELF::STT_SECTION`, which returns the name of the section from the section string table.
Philip Reames [Thu, 31 Jan 2019 18:45:46 +0000 (18:45 +0000)]
Lower widenable_conditions in CGP
This ensures that if we make it to the backend w/o lowering widenable_conditions first, that we generate correct code. Doing it in CGP - instead of isel - let's us fold control flow before hitting block local instruction selection.
Bob Wilson [Thu, 31 Jan 2019 17:58:59 +0000 (17:58 +0000)]
[ADT] Fix a typo in isOSVersionLT that breaks the Micro version check
The original commit of this function (r129800 in 2011) had a typo where
part of the "Micro" version check was actually comparing against the "Minor"
version number.
Similar to what we already do in DAGCombiner, but this version also handles bitcasts from types with different scalar sizes, which x86 is better at handling.
Teresa Johnson [Thu, 31 Jan 2019 16:46:14 +0000 (16:46 +0000)]
Revert "[ThinLTO] Rename COMDATs for COFF when promoting/renaming COMDAT leader"
This reverts commit r352763.
Causing a couple bot failures, root cause pointed to by sanitizer bot:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/28909/steps/annotate/logs/stdio
Use after free. I understand the issue but will revert and test with fix
before recommitting.
Jordan Rupprecht [Thu, 31 Jan 2019 16:45:16 +0000 (16:45 +0000)]
[llvm-objcopy] Skip --localize-symbol for undefined symbols
Summary:
Include the symbol being defined in the list of requirements for using --localize-symbol.
This is used, for example, when someone is depending on two different projects that have the same (or close enough) method defined in each library, and using "-L sym" for a conflicting symbol in one of the libraries so that the definition from the other one is used. However, the library may have internal references to the symbol, which cause program crashes when those are used, i.e.:
```
$ cat foo.c
int foo() { return 5; }
$ cat bar.c
int foo();
int bar() { return 2 * foo(); }
$ cat foo2.c
int foo() { /* Safer implementation */ return 42; }
$ cat main.c
int bar();
int main() {
__builtin_printf("bar = %d\n", bar());
return 0;
}
$ ar rcs libfoo.a foo.o bar.o
$ ar rcs libfoo2.a foo2.o
# Picks the wrong foo() impl
$ clang main.o -lfoo -lfoo2 -L. -o main
# Picks the right foo() impl
$ objcopy -L foo libfoo.a && clang main.o -lfoo -lfoo2 -L. -o main
# Links somehow, but crashes at runtime
$ llvm-objcopy -L foo libfoo.a && clang main.o -lfoo -lfoo2 -L. -o main
```
Sanjay Patel [Thu, 31 Jan 2019 16:40:07 +0000 (16:40 +0000)]
[PatternMatch] add special-case uaddo matching for increment-by-one
This is the most important uaddo problem mentioned in PR31754:
https://bugs.llvm.org/show_bug.cgi?id=31754
We were failing to match the canonicalized pattern when it's an 'add 1' operation.
Pattern matching, however, shouldn't assume that we have canonicalized IR, so we
match 4 commuted variants of uaddo.
There's also a test with a crazy type to show that the existing CGP transform
based on this matcher is not limited by target legality checks, but that's a
different problem.
Teresa Johnson [Thu, 31 Jan 2019 16:00:15 +0000 (16:00 +0000)]
[ThinLTO] Rename COMDATs for COFF when promoting/renaming COMDAT leader
Summary:
COFF requires that COMDAT name match that of the leader. When we promote
and rename an internal leader in ThinLTO due to an import, ensure we
subsequently rename the associated COMDAT. Similar to D31963 which did
this during ThinLTO module splitting.
James Henderson [Thu, 31 Jan 2019 14:22:50 +0000 (14:22 +0000)]
[llvm-symbolizer][test] Extract tests from llvm-symbolizer.test and simplify (#3)
This is the fourth (and final for now) of a series of patches
simplifying llvm-symbolizer tests. See r352752, r352753 and 352754 for
the previous ones. This patch splits out several more distinct test
cases from llvm-symbolizer.test into separate tests, and simplifies them
in various ways including:
1) Building a test case for spaces in path from source, rather than
using a pre-canned binary. This allows deleting of said binary and the
source it was built from.
2) Switching to specifying addresses and objects directly on the
command-line rather than via stdin.
This also adds an explict test for the ability to specify a file and
address as a line in stdin, since the majority of the tests have been
migrated away from this approach, leaving this largely untested.
James Henderson [Thu, 31 Jan 2019 14:17:33 +0000 (14:17 +0000)]
[llvm-symbolizer][test] Extract tests from llvm-symbolizer.test and simplify (#2)
This is the third of a series of patches simplifying llvm-symbolizer
tests. See r352752 and r352753 for the previous two. This patch splits
out a number of distinct test cases from llvm-symbolizer.test into
separate tests, and simplifies them in various ways including:
1) using --obj/positional arguments for the input file and addresses
instead of stdin,
2) using runtime-generated inputs rather than a pre-canned binary, and
3) testing more specifically (i.e. checking only what is interesting to
the behaviour changed in the original commit for that test case).
This patch also removes the test case for using --obj. The
tools/llvm-symbolizer/basic.s test already tests this case. Finally,
this patch adds a simple test case to the demangle switch test case to
show that demangling happens by default.
See https://bugs.llvm.org/show_bug.cgi?id=40070#c1 for the motivation.
James Henderson [Thu, 31 Jan 2019 14:11:17 +0000 (14:11 +0000)]
[llvm-symbolizer][test] Extract tests from llvm-symbolizer.test and simplify (#1)
This is the second of a series of patches simplifying llvm-symbolizer
tests. See r352752 for the first. This one splits out 5 distinct test
cases from llvm-symbolizer.test into separate tests, and simplifies them
slightly by using --obj/positional arguments for the input file and
addresses instead of stdin.
See https://bugs.llvm.org/show_bug.cgi?id=40070#c1 for the motivation.
James Henderson [Thu, 31 Jan 2019 14:04:47 +0000 (14:04 +0000)]
[llvm-symbolizer][test] Simplify test input reading
This change migrates most llvm-symbolizer tests away from reading input
via stdin and instead using --obj + positional arguments for the file
and addresses respectively, which makes the tests easier to read.
One exception is the test test/tools/llvm-symbolizer/pdb/pdb.test, which
was doing some manipulation on the input addresses. This patch
simplifies this somewhat, but it still reads from stdin.
More changes to follow to simplify/break-up other tests.
James Henderson [Thu, 31 Jan 2019 13:58:48 +0000 (13:58 +0000)]
[CommandLine] Improve help text for cl::values style options
In order to make an option value truly optional, both the ValueOptional
and an empty-named value are required. This empty-named value appears in
the command-line help text, which is not ideal.
This change improves the help text for these sort of options in a number
of ways:
1) ValueOptional options with an empty-named value now print their help
text twice: both without and then with '=<value>' after the name. The
latter version then lists the allowed values after it.
2) Empty-named values with no help text in ValueOptional options are not
listed in the permitted values.
3) Otherwise empty-named options are printed as =<empty> rather than
simply '='.
4) Option values without help text do not have the '-' separator
printed.
It also tweaks the llvm-symbolizer -functions help text to not print a
trailing ':' as that looks bad combined with 1) above.
Sjoerd Meijer [Thu, 31 Jan 2019 08:38:06 +0000 (08:38 +0000)]
[ARM] Thumb2: ConstantMaterializationCost
Constants can also be materialised using the negated value and a MVN, and this
case seem to have been missed for Thumb2. To check the constant materialisation
costs, we now call getT2SOImmVal twice, once for the original constant and then
also for its negated value, and this function checks if the constant can both
be splatted or rotated.
This was revealed by a test that optimises for minsize: instead of a LDR
literal pool load and having a literal pool entry, just a MVN with an immediate
is smaller (and also faster).
Sjoerd Meijer [Thu, 31 Jan 2019 08:07:30 +0000 (08:07 +0000)]
[SelectionDAG] Codesize: don't expand SHIFT to SHIFT_PARTS
And instead just generate a libcall. My motivating example on ARM was a simple:
shl i64 %A, %B
for which the code bloat is quite significant. For other targets that also
accept __int128/i128 such as AArch64 and X86, it is also beneficial for these
cases to generate a libcall when optimising for minsize. On these 64-bit targets,
the 64-bits shifts are of course unaffected because the SHIFT/SHIFT_PARTS
lowering operation action is not set to custom/expand.
Douglas Yung [Thu, 31 Jan 2019 07:58:34 +0000 (07:58 +0000)]
Fixup test after r352704 since it changes how paths may be emitted.
On Unix/Mac OS X, normpath() returns the path unchanged (FileCheck), but
on case-insensitive filesystems (like NTFS on Windows), it converts the
path to lowercase (filecheck) which was causing the test to fail.
Petr Hosek [Thu, 31 Jan 2019 06:21:01 +0000 (06:21 +0000)]
[CMake] Unify scripts for generating VCS headers
Previously, there were two different scripts for generating VCS headers:
one used by LLVM and one used by Clang. They were both similar, but
different. They were both broken in their own ways, for example the one
used by Clang didn't properly handle monorepo resulting in an incorrect
version information reported by Clang.
This change unifies two the scripts by introducing a new script that's
used from both LLVM and Clang, ensures that the new script supports both
monorepo and standalone SVN and Git setups, and removes the old scripts.
Max Kazantsev [Thu, 31 Jan 2019 06:19:25 +0000 (06:19 +0000)]
[SCEV] Prohibit SCEV transformations for huge SCEVs
Currently SCEV attempts to limit transformations so that they do not work with
big SCEVs (that may take almost infinite compile time). But for this, it uses heuristics
such as recursion depth and number of operands, which do not give us a guarantee
that we don't actually have big SCEVs. This situation is still possible, though it is not
likely to happen. However, the bug PR33494 showed a bunch of simple corner case
tests where we still produce huge SCEVs, even not reaching big recursion depth etc.
This patch introduces a concept of 'huge' SCEVs. A SCEV is huge if its expression
size (intoduced in D35989) exceeds some threshold value. We prohibit optimizing
transformations if any of SCEVs we are dealing with is huge. This gives us a reliable
check that we don't spend too much time working with them.
As the next step, we can possibly get rid of old limiting mechanisms, such as recursion
depth thresholds.
Nico Weber [Thu, 31 Jan 2019 00:40:43 +0000 (00:40 +0000)]
lit: Let lit.util.which() return a normcase()ed path
LLVMConfig.with_environment() uses os.path.normcase(os.path.normpath(x)) to
normalize temporary env vars. LLVMConfig.use_clang() uses with_environment() to
temporarily set PATH and then look for clang there. This means that on Windows,
clang will be run with a path like c:\foo\bin\clang.EXE (with a lower-case
"C:").
lit.util.which() used to not do this, which means the executables added in
clang/test/lit.cfg.py (e.g. c-index-test) were run with a path like
C:\foo\bin\c-index-test.EXE (because both CMake and GN happen to write
clang_tools_dir with an upper-case C to lit.site.cfg.py).
clang/test/Index/pch-from-libclang.c requires that both c-index-test and clang
use _exactly_ the same resource dir path (same case and everything), because a
hash of the resource directory is used as module cache path.
This patch is necessary but not sufficient to make pch-from-libclang.c pass on
Windows.
Thomas Lively [Thu, 31 Jan 2019 00:35:37 +0000 (00:35 +0000)]
[LegalizeVectorTypes] Allow illegal indices when splitting extract_vector_elt
Summary:
Fixes PR40267, in which the removed assertion was triggering on
perfectly valid IR. As far as I can tell, constant out of bounds
indices should be allowed when splitting extract_vector_elt, since
they will simply be propagated as out of bounds indices in the
resulting split vector and handled appropriately elsewhere.
Craig Topper [Thu, 31 Jan 2019 00:04:48 +0000 (00:04 +0000)]
[LegalizeTypes] Use report_fatal_error instead of llvm_unreachable in the default case of some type legalization handlers that can be reached with intrinsics with result or operands that aren't legal types.
These can be triggered by mistakenly using a 64-bit mode only intrinsics with a -mtriple=i686. Using report_fatal_error gives a better experience for this mistake in release builds instead of probably crashing.
We already do this for some of the vector type legalization handles.
Craig Topper [Thu, 31 Jan 2019 00:04:46 +0000 (00:04 +0000)]
[X86] Remove handling of ISD::INTRINSIC_WO_CHAIN in ReplaceNodeResults.
I believe this was there to handle avx512bw intrinsics that returned i64 type in 32-bit mode. But all those intrinsics have since been changed to v64i1 results or replaced with generic IR.
Heejin Ahn [Wed, 30 Jan 2019 23:53:36 +0000 (23:53 +0000)]
[WebAssembly] Remove TODO on wasm.extract.exception intrinsic (NFC)
Summary:
We planned to delete this intrinsic and do custom lowering from
`wasm.get.exception`, which has a token argument, to
`EXTRACT_EXCEPTION`, a wasm pseudo instruction that simulates popping a
value from the wasm stack.
To do that, we need to introduce a new `WebAssemblyISD` node for this,
which itself is not a problem, but also have to introduce the
`WebAssemblyISD` namespace in SelectionDAGBuilder.cpp. I don't think any
other targets are doing that in the file. And also putting a
target-specific intrinsic in the common file is a little weird too. (All
other intrinsic functions in this `visitIntrinsicCall` functions are not
target-specific ones. Other target-specific intrinsics are usually
handled in `lib/Target/[TargetName]/[TargetName]ISelLowering.cpp`. The
reason we can't do this is it has a token argument.
Anyway, so I think I prefer the current code with one redundant
intrinsic more than adding one more `WebAssemblyISD` node and
also introducing the `WebAssemblyISD` namespace into
SelectionDAGBuilder.cpp. What do you think?
Zachary Turner [Wed, 30 Jan 2019 23:52:32 +0000 (23:52 +0000)]
[RuntimeDyld] Don't try to allocate sections with align 0.
ELF sections allow 0 for the alignment, which is specified to
be the same as 1. However many clients do not expect this and
will behave poorly in the presence of a 0-aligned section (for
example by trying to modulo something by the section alignment).
We can be more polite by making sure that we always pass a
non-zero value to clients.
projects / llvm / log