Matt Arsenault [Mon, 15 Jul 2019 21:41:44 +0000 (21:41 +0000)]
AMDGPU: Use standalone MUBUF load patterns
We already do this for the flat and DS instructions, although it is
certainly uglier and more verbose.
This will allow using separate pattern definitions for extload and
zextload. Currently we get away with using a single PatFrag with
custom predicate code to check if the extension type is a zextload or
anyextload. The generic mechanism the global isel emitter understands
treats these as mutually exclusive. I was considering making the
pattern emitter accept zextload or sextload extensions for anyextload
patterns, but in global isel, the different extending loads have
distinct opcodes, and there is currently no mechanism for an opcode
matcher to try multiple (and there probably is very little need for
one beyond this case).
Nick Desaulniers [Mon, 15 Jul 2019 21:16:29 +0000 (21:16 +0000)]
[LoopUnroll+LoopUnswitch] do not transform loops containing callbr
Summary:
There is currently a correctness issue when unrolling loops containing
callbr's where their indirect targets are being updated correctly to the
newly created labels, but their operands are not. This manifests in
unrolled loops where the second and subsequent copies of callbr
instructions have blockaddresses of the label from the first instance of
the unrolled loop, which would result in nonsensical runtime control
flow.
For now, conservatively do not unroll the loop. In the future, I think
we can pursue unrolling such loops provided we transform the cloned
callbr's operands correctly.
Such a transform and its legalities are being discussed in:
https://reviews.llvm.org/D64101
Matt Arsenault [Mon, 15 Jul 2019 21:15:20 +0000 (21:15 +0000)]
TableGen/GlobalISel: Fix handling of truncstore patterns
This was failing to import the AMDGPU truncstore patterns. The
truncating stores from 32-bit to 8/16 were then somehow being
incorrectly selected to a 4-byte store.
A separate check is emitted for the LLT size in comparison to the
specific memory VT, which looks strange to me but makes sense based on
the hierarchy of PatFrags used for the default truncstore PatFrags.
Matt Arsenault [Mon, 15 Jul 2019 20:59:42 +0000 (20:59 +0000)]
TableGen: Add address space to matchers
Currently AMDGPU uses a CodePatPred to check address spaces from the
MachineMemOperand. Introduce a new first class property so that the
existing patterns can be easily modified to uses the new generated
predicate, which will also be handled for GlobalISel.
I would prefer these to match against the pointer type of the
instruction, but that would be difficult to get working with
SelectionDAG compatbility. This is much easier for now and will avoid
a painful tablegen rewrite for all the loads and stores.
I'm also not sure if there's a better way to encode multiple address
spaces in the table, rather than putting the number to expect.
Matt Arsenault [Mon, 15 Jul 2019 20:20:18 +0000 (20:20 +0000)]
AMDGPU/GlobalISel: Allow scalar s1 and/or/xor
If a 1-bit value is in a 32-bit VGPR, the scalar opcodes set SCC to
whether the result is 0. If the inputs are SCC, these can be copied to
a 32-bit SGPR to produce an SCC result.
Add "memtag" sanitizer that detects and mitigates stack memory issues
using armv8.5 Memory Tagging Extension.
It is similar in principle to HWASan, which is a software implementation
of the same idea, but there are enough differencies to warrant a new
sanitizer type IMHO. It is also expected to have very different
performance properties.
The new sanitizer does not have a runtime library (it may grow one
later, along with a "debugging" mode). Similar to SafeStack and
StackProtector, the instrumentation pass (in a follow up change) will be
inserted in all cases, but will only affect functions marked with the
new sanitize_memtag attribute.
Matt Arsenault [Mon, 15 Jul 2019 19:48:36 +0000 (19:48 +0000)]
AMDGPU/GlobalISel: Don't constrain source register of VCC copies
This is a hack until I come up with a better way of dealing with the
pseudo-register banks used for boolean values. If the use instruction
constrains the register, the selector for the def instruction won't
see that the bank was VCC. A 1-bit SReg_32 is could ambiguously have
been SCCRegBank or VCCRegBank in wave32.
This is necessary to successfully select branches with and and/or/xor
condition.
Matt Arsenault [Mon, 15 Jul 2019 19:46:48 +0000 (19:46 +0000)]
AMDGPU/GlobalISel: Fix selecting vcc->vcc bank copies
The extra test change is correct, although how it arrives there is a
bug that needs work. With wave32, the test for isVCC ambiguously
reports true for an SCC or VCC source. A new allocatable pseudo
register class for SCC may be necesssary.
Summary:
Processing of command-line definition of variable and logic around
implicit not directives both reuse parsing code that expects a line
number to be defined. So far, a special line number of 0 was used for
those users of the parsing code where a line number does not make sense.
This commit instead represents line numbers as Optional values so that
they can be None for those cases.
[cmake] Don't set install rules for tblgen if building utils is disabled
Summary:
This is a follow up to D64032. Afterwards if building utils is disabled
and cross compilation is attempted, CMake will complain that adding
`install()` directives to targets with EXCLUDE_FROM_ALL set is "undefined".
Indeed, it appears depending on the CMake version and the selected
Generator, the install rule will error because the underlying target isn't
built. Fix that by not adding the install rule if building utils is not
requested. Note that this doesn't prevent building tblgen as a
dependency in not cross-build, even if building tools is disabled.
Expand comment about how StringsToBuckets was computed, and add more entries
The construction was explained in
https://reviews.llvm.org/D44810?id=139526#inline-391999 but reading the code
shouldn't require hunting down old reviews to understand it.
The precomputed list was missing an entry for the empty list case, and
one entry at the very end. (The current last entry is the last one where
3 * BucketCount fits in a signed int, but the reference implementation
uses unsigneds as far as I can tell, so there's room for one more entry.)
David Green [Mon, 15 Jul 2019 18:42:54 +0000 (18:42 +0000)]
[ARM] MVE vector for 64bit types
We need to make sure that we are sensibly dealing with vectors of types v2i64
and v2f64, even if most of the time we cannot generate native operations for
them. This mostly adds a lot of testing, plus fixes up a couple of the issues
found. And, or and xor can be legal for v2i64, and shifts combining needs a
slight fixup.
We have pseudo-vectorization of scalar int to FP casts, so this tries to
make that more likely by replacing a truncate with a bitcast. I didn't see
any test diffs starting from 'uitofp', so I left that as a TODO. We can't
only match the shorter trunc+extract pattern because there's an opposing
transform somewhere, so we infinite loop. Waiting to try this during
lowering is another possibility.
A motivating case is shown in PR39975 and included in the test diffs here:
https://bugs.llvm.org/show_bug.cgi?id=39975
[llvm-lib] Add a dependency to intrinsics_gen to the LLVMLibDriver build
Summary:
Occasionally the build of LLVMLibDriver will fail because Attributes.inc has not been generated yet. Add an explicit dependency, so that we can guarantee that the file has been generated before LLVMLibDriver is build.
##[error]llvm\include\llvm\IR\Attributes.h(73,0): Error C1083: Cannot open include file: 'llvm/IR/Attributes.inc': No such file or directory
llvm\include\llvm/IR/Attributes.h(73): fatal error C1083: Cannot open include file: 'llvm/IR/Attributes.inc': No such file or directory [LLVMLibDriver.vcxproj]
[X86] Return UNDEF from LowerScalarImmediateShift when the shift amount is out of range.
I think we only turn out of range shiftss to undef when
all elements are out of range or the shift amount is a splat out
of range. I'm not sure which, I didn't check.
During lowering we can split a shift where some elements
are out of range into multiple shifts. This can create a
new shift with a splat shift amount that is out of range.
Matt Arsenault [Mon, 15 Jul 2019 17:50:31 +0000 (17:50 +0000)]
AMDGPU: Add 24-bit mul intrinsics
Insert these during codegenprepare.
This works around a DAG issue where generic combines eliminate the and
asserting the high bits are zero, which then exposes an unknown read
source to the mul combine. It doesn't worth the hassle of trying to
insert an AssertZext or something to try to deal with it.
[FunctionAttrs] Remove readonly and writeonly assertion
There are scenarios where mutually recursive functions may cause the SCC
to contain both read only and write only functions. This removes an
assertion when adding read attributes which caused a crash with a the
provided test case, and instead just doesn't add the attributes.
Having the hash table key change during iteration is bad, so make it
impossible. Nothing relied on the key type not being const.
(This is also necessary to be able to call the const version of
iterator_facade_base::operator->(). Nothing calls this, and nothing
will, but I tried using it locally during development and it took me a
while to understand what was going wrong.)
Also rename the iterator typedef to const_iterator.
ELF.h contains two getSymbol methods
which seems to be used only from obj2yaml.
One of these methods calls another, which in turn
contains untested error message which doesn't
provide enough information.
Problem is that after improving only just that message,
obj2yaml will not show it,
("Error reading file: yaml: Invalid data was
encountered while parsing the file" message will be shown instead),
because internal errors handling of tool is based on ErrorOr<> class which
stores a error code and as a result can only show a predefined error string, what
actually isn't very useful.
In this patch, I rework obj2yaml's error reporting system
for ELF targets to use Error Expected<> classes.
Also, I improve the error message produced
by getSymbol for demonstration of the new functionality.
David Green [Mon, 15 Jul 2019 11:35:39 +0000 (11:35 +0000)]
[ARM] MVE Vector Shifts
This adds basic lowering for MVE shifts. There are many shifts in MVE, but the
instructions handled here are:
VSHL (imm)
VSHRu (imm)
VSHRs (imm)
VSHL (vector)
VSHL (register)
MVE, like NEON before it, doesn't have shift right by a vector (or register).
We instead have to negate the amount and shift in the opposite direction. This
means we have to convert any SHR's into a form of SHL (that is still signed or
unsigned) with a negated condition and selecting from there. MVE still does
have shifting by an immediate for SHL, ASR and LSR.
This adds lowering for these and for register forms, which work well for shift
lefts but may require an extra fold of neg(vdup(x)) -> vdup(neg(x)) to potentially
work optimally for right shifts.
George Rimar [Mon, 15 Jul 2019 10:50:03 +0000 (10:50 +0000)]
[obj2yaml] - Rework tool's error reporting logic for ELF target.
ELF.h contains two getSymbol methods
which seems to be used only from obj2yaml.
One of these methods calls another, which in turn
contains untested error message which doesn't
provide enough information.
Problem is that after improving only just that message,
obj2yaml will not show it,
("Error reading file: yaml: Invalid data was
encountered while parsing the file" message will be shown instead),
because internal errors handling of tool is based on ErrorOr<> class which
stores a error code and as a result can only show a predefined error string, what
actually isn't very useful.
In this patch, I rework obj2yaml's error reporting system
for ELF targets to use Error Expected<> classes.
Also, I improve the error message produced
by getSymbol for demonstration of the new functionality.
David Green [Mon, 15 Jul 2019 10:44:50 +0000 (10:44 +0000)]
[ARM] Adjust how NEON shifts are lowered
This adjusts the way that we lower NEON shifts to use a DAG target node, not
via a neon intrinsic. This is useful for handling MVE shifts operations in the
same the way. It also renames some of the immediate shift nodes for
consistency, and moves some of the processing of immediate shifts into
LowerShift allowing it to capture more cases.
[Loop Peeling] Fix the bug with IDom setting for exit loops
It is possible that loop exit has two predecessors in a loop body.
In this case after the peeling the iDom of the exit should be a clone of
iDom of original exit but no a clone of a block coming to this exit.
[LoopUtils] Extend the scope of getLoopEstimatedTripCount
With this patch the getLoopEstimatedTripCount function will
accept also the loops where there are more than one exit but
all exits except latch block should ends up with a call to deopt.
This side exits should not impact the estimated trip count.
Yonghong Song [Mon, 15 Jul 2019 04:51:34 +0000 (04:51 +0000)]
[BPF] add unit tests for preserve_{array,union,struct}_access_index intrinsics
This is a followup patch for https://reviews.llvm.org/D61810/new/,
which adds new intrinsics preserve_{array,union,struct}_access_index.
Currently, only BPF backend utilizes preserve_{array,union,struct}_access_index
intrinsics, so all tests are compiled with BPF target.
https://reviews.llvm.org/D61524 already added some tests for these
intrinsics, but some of them pretty complex.
This patch added a few unit test cases focusing on individual intrinsic
functions.
Also made a few clarification on language reference for these intrinsics.
[X86] Separate the memory size of vzext_load/vextract_store from the element size of the result type. Use them improve the codegen of v2f32 loads/stores with sse1 only.
Summary:
SSE1 only supports v4f32. But does have instructions like movlps/movhps that load/store 64-bits of memory.
This patch breaks the connection between the node VT of the vzext_load/vextract_store patterns and the memory VT. Enabling a v4f32 node with a 64-bit memory VT. I've used i64 as the memory VT here. I've written the PatFrag predicate to just check the store size not the specific VT. I think the VT will only matter for CSE purposes. We could use v2f32, but if we want to start using these operations in more places a simple integer type might make the most sense.
I'd like to maybe use this same thing for SSE2 and later as well, but that will need more work to be supported by EltsFromConsecutiveLoads to avoid regressing lit tests. I'd maybe also like to combine bitcasts with these load/stores nodes now that the types are disconnected. And I'd also like to consider canonicalizing (scalar_to_vector + load) to vzext_load.
If you want I can split the mechanical tablegen stuff where I added the 32/64 off from the sse1 change.
[TargetParser][ARM] Account dependencies when processing target features
Teaches ARM::appendArchExtFeatures to account dependencies when processing
target features: i.e. when you say -march=armv8.1-m.main+mve.fp+nofp it
means mve.fp should get discarded too. (Split from D63936)
[LV] Exclude loop-invariant inputs from scalar cost computation.
Loop invariant operands do not need to be scalarized, as we are using
the values outside the loop. We should ignore them when computing the
scalarization overhead.
[clang][Driver][ARM] Favor -mfpu over default CPU features
When processing the command line options march, mcpu and mfpu, we store
the implied target features on a vector. The change D62998 introduced a
temporary vector, where the processed features get accumulated. When
calling DecodeARMFeaturesFromCPU, which sets the default features for
the specified CPU, we certainly don't want to override the features
that have been explicitly specified on the command line. Therefore, the
default features should appear first in the final vector. This problem
became evident once I added the missing (unhandled) target features in
ARM::getExtensionFeatures.
[BitcodeReader] Use tighter upper bound to validate forward references.
At the moment, bitcode files with invalid forward reference can easily
cause the bitcode reader to run out of memory, by creating a forward
reference with a very high index.
We can use the size of the bitcode file as an upper bound, because a
valid bitcode file can never contain more records. This should be
sufficient to fail early in most cases. The only exception is large
files with invalid forward references close to the file size.
There are a couple of clusterfuzz runs that fail with out-of-memory
because of very high forward references and they should be fixed by this
patch.
A concrete example for this is D64507, which causes out-of-memory on
systems with low memory, like the hexagon upstream bots.
David Green [Sat, 13 Jul 2019 15:43:00 +0000 (15:43 +0000)]
[ARM] Add sign and zero extend patterns for MVE
The vmovlb instructions can be uses to sign or zero extend vector registers
between types. This adds some patterns for them and relevant testing. The
VBICIMM generation is also put behind a hasNEON check (as is already done for
VORRIMM).
David Green [Sat, 13 Jul 2019 14:38:53 +0000 (14:38 +0000)]
[ARM] MVE VRINT support
This adds support for the floor/ceil/trunc/... series of instructions,
converting to various forms of VRINT. They use the same suffixes as their
floating point counterparts. There is not VTINTR, so nearbyint is expanded.
Also added a copysign test, to show it is expanded.
FileCheck [7/12]: Arbitrary long numeric expressions
Summary:
This patch is part of a patch series to add support for FileCheck
numeric expressions. This specific patch extend numeric expression to
support an arbitrary number of operands, either variable or literals.
Copyright:
- Linaro (changes up to diff 183612 of revision D55940)
- GraphCore (changes in later versions of revision D55940 and
in new revision created off D55940)
Revert "[InstCombine] add tests for umin/umax via usub.sat; NFC"
This reverts commit rL365999 / 0f6148df23edcd3081f5e761de19edd4f823f16d.
The tests already exist in this file, and the hoped-for transform
(mentioned in D62871) is invalid because of undef as discussed in
D63060.
[Attributor] Only return attributes with a valid state
Attributor::getAAFor will now only return AbstractAttributes with a
valid AbstractState. This simplifies call sites as they only need to
check if the returned pointer is non-null. It also reduces the potential
for accidental misuse.