LLVM and SPIRV-LLVM-Translator pulldown (WW37) #2432
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Handling the new min/max intrinsics is the motivation, but it turns out that we have a bunch of other intrinsics with this missing bit of analysis too. The FP min/max tests show that we are intersecting FMF, so that part should be safe too. As noted in https://llvm.org/PR46897 , there is a commutative property specifier for intrinsics, but no corresponding function attribute, and so apparently no uses of that bit. We may want to remove that next. Follow-up patches should wire up the Instruction::isCommutative() to this IntrinsicInst specialization. That requires updating callers to be aware of the more general commutative property (not just binops). Differential Revision: https://reviews.llvm.org/D86798
…ing changes The stage2-stage3 differences persist even without instcombine-based PHI CSE, so this is the only possible reason.
Not sure if this has any effect, but it was inconsistent before. Reviewed By: rsmith Differential Revision: https://reviews.llvm.org/D67113
Instead of writing to a flag and then returning based on that flag we can also return directly. The flag name also doesn't provide additional information, it just reflects the name of the function (isReferenced).
A MemoryPhi can never be eliminated. If we hit one, return the Phi, so the caller can continue traversing the incoming accesses. This saves some unnecessary read clobber checks and improves compile-time http://llvm-compile-time-tracker.com/compare.php?from=1ffc58b6d098ce8fa71f3a80fe75b990f633f921&to=d0fa8d1982380b57d7b6067528104bc373dbe07a&stat=instructions
… to undef This patch fixes AANoUndef manifestation. We should not manifest noundef for positions that will be changed to undef. Reviewed By: jdoerfert Differential Revision: https://reviews.llvm.org/D86835
This got reverted because a dependency was reverted. It has since been reapplied, so reapply this as well. ----- Related to D69686. As noted there, LVI currently behaves differently for integer and pointer values: For integers, the block value is always valid inside the basic block, while for pointers it is only valid at the end of the basic block. I believe the integer behavior is the correct one, and CVP relies on it via its getConstantRange() uses. The reason for the special pointer behavior is that LVI checks whether a pointer is dereferenced in a given basic block and marks it as non-null in that case. Of course, this information is valid only after the dereferencing instruction, or in conservative approximation, at the end of the block. This patch changes the treatment of dereferencability: Instead of including it inside the block value, we instead treat it as something similar to an assume (it essentially is a non-nullness assume) and incorporate this information in intersectAssumeOrGuardBlockValueConstantRange() if the context instruction is the terminator of the basic block. This happens either when determining an edge-value internally in LVI, or when a terminator was explicitly passed to getValueAt(). The latter case makes this more powerful than the previous implementation as a side-effect, and this does actually seem benefitial in practice. Of course, we do not want to recompute dereferencability on each intersectAssume call, so we need a new cache for this. The dereferencability analysis requires walking the entire basic block and computing underlying objects of all memory operands. This was previously done separately for each queried pointer value. In the new implementation (both because this makes the caching simpler, and because it is faster), I instead only walk the full BB once and cache all the dereferenced pointers. So the traversal is now performed only once per BB, instead of once per queried pointer value. I think the overall model now makes more sense than before, and there will be no more pitfalls due to differing integer/pointer behavior. Differential Revision: https://reviews.llvm.org/D69914
Move bail out when optimizing for size before runtime check generation. In that case, we do not use the result of the expansion, the expanded instruction will be dead and cleaned up later. By doing the check before expanding the runtime-checks, we can save a bit of unnecessary work.
…(PR47322) Replace the check for poison-producing instructions in SimplifyWithOpReplaced() with the generic helper canCreatePoison() that properly handles poisonous shifts and thus avoids the problem from PR47322. This additionally fixes a bug in IIQ.UseInstrInfo=false mode, which previously could have caused this code to ignore poison flags. Setting UseInstrInfo=false should reduce the possible optimizations, not increase them. This is not a full solution to the problem, as poison could be introduced more indirectly. This is just a minimal, easy to backport fix. Differential Revision: https://reviews.llvm.org/D86834
…(NFC) Canonicalize icmp ne to icmp eq and implement all the folds only once.
Even though `noundef` IR attribute might be attached to non-void type values, AANoUndef is mistakenly identified for pointer type values only. This patch fixes that. Reviewed By: jdoerfert Differential Revision: https://reviews.llvm.org/D86737
This commit adds the first from-scratch configuration files for running the libc++ test suite without using the old configuration: - libcxx-trunk-shared.cfg.py: Runs the test suite against a trunk libc++ shared library. - libcxx-trunk-static.cfg.py: Runs the test suite against a trunk libc++ static library. Differential Revision: https://reviews.llvm.org/D81866
This ensures that existing CMake build trees will start using the new default without having to nuke their build directories.
DFS and Reverse-DFS linkage orders are used to order execution of deinitializers and initializers respectively. This patch replaces uses of special purpose DFS order functions in MachOPlatform and LLJIT with uses of the new methods.
Otherwise their alignment is dependent on the size of the section. If the size is large than 16, the alignment will be 16. 16 is a bad choice for both .llvmbc and .llvmcmd because the padding between two contributions from input sections is of a variable size. A bitstream is actually guaranteed to be 4-byte aligned, but consumers don't need this property.
…ateImpl This patch fixes wrong dependency type in AAUB. Reviewed By: jdoerfert Differential Revision: https://reviews.llvm.org/D86842
This is the next patch of D86842 When we check `noundef` attribute violation at callsites, we do not have to require `nonnull` in the following two cases. 1. An argument is known to be simplified to undef 2. An argument is known to be dead Reviewed By: jdoerfert Differential Revision: https://reviews.llvm.org/D86845
This patch makes LangRef be explicit about the value of padding when storing an aggregate. It states that when an aggregate is stored into memory, padding is filled with undef. Here is a clue that supports this change (edited to reflect the discussion from llvm-dev): - IPSCCP ignores padding and directly stores a constant aggregate if possible. It loses the data stored in the padding. https://godbolt.org/z/xzenYs Memcpyopt ignores (the preexisting value of) padding when copying an aggregate or storing a constant: https://godbolt.org/z/hY6ndd / https://godbolt.org/z/3WMP5a The two items below are not relevant with this patch because Clang lowers load/store of individual field of struct into load/stores of the corresponding pointer with a primitive type. Also, when copy is needed, it uses memcpy instead of load/store of an aggregate, as discussed in the llvm-dev. However, this patch is still valid (as discussed) because it is needed to explain the two optimizations above. - According to C17, the value of padding bytes when storing values in structures or unions is unspecified. - I updated Alive2 and it did not find any problematic transformation from LLVM unit tests and while running translation validation of a few C programs. Reviewed By: nikic Differential Revision: https://reviews.llvm.org/D86189
Differential Revision: https://reviews.llvm.org/D86848
Adjusted the test in accordance with llvm/llvm-project@57d8aca
UIToFPInst LLVM instruction is translated to OpConvertUToF SPIRV instruction. OpConvertUToF instruction cannot receive boolean operand, so replace boolean operand with with select instruction.
Added an error message when the translator encounters an unknow/unsupported LLVM instruction
They are deprecated in the later version of the spec. Consumption of them is saved for backward compatibility.
In all cases what we have encountered so far each instruction has either zero or one extension that it requires and therefore there is no need to use `std::set` to store that information. Replaced `std::set` with `llvm::Optional` and renamed function to indicate that it can only return a single extension.
The assertion now gives way to a simple check until all known LLVM IR edge cases that do not accord to the condition can be handled. Signed-off-by: Artem Gindinson <[email protected]>
SPIRVTypeImage uses std::vector<T> emulating std::optional behavior This commit allows to use native llvm::Optional class in SPIRVEncoder and SPIRVDecoder and replace std::vector in SPIRVTypeImage
Fixed an error caused by interference of KhronosGroup/SPIRV-LLVM-Translator@7cd1a3b and KhronosGroup/SPIRV-LLVM-Translator@584329f
This reverts commit 2377920.
|
/summary:run |
|
Waiting for Jenkins/Summary completion run for 8736025. |
vladimirlaz
requested review from
AGindinson,
AlexeySachkov,
AlexeySotkin,
bader,
elizabethandrews,
Fznamznon,
mdtoguchi,
premanandrao and
a team
as code owners
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
LLVM: llvm/llvm-project@c4a2a13
SPIRV-LLVM-Translator: KhronosGroup/SPIRV-LLVM-Translator@537218d