Specify that standard conversion rules do not apply to assignments for matrix types.

In D70456#1959561, @LuoYuanke wrote:

I have the similar question on how to lower matrix intrinsics to some HW specific intrinsics/instruction. For example, X86 have the AVX512_VNNI feature (https://software.intel.com/sites/landingpage/IntrinsicsGuide/#expand=39,5370,5361,364,142,139,2210&text=vnni). It can perform dot product computation. But after matrix intrinsic is lowered to vector, it seems difficult to transform the vector operation to AVX512_VNNI intrinsic/instruction.

Hi Renato,

Use 20 bits in MatrixTypeBitfields for both number of rows and number of columns. This leaves 24 bits for NumTypeBits, while providing a large ranges for number of rows/columns.

Fix wrong formatting of check lines..

In D77363#1959149, @vitalybuka wrote:

This does not work for me

In D73763#1956347, @george.burgess.iv wrote:

I think there are 2 cases to distinguish:

1. For accesses to non-alloca objects, this matches what the intrinsic achieves I think. We can only eliminate stores to objects that are visible after the function returns, if they are overwritten along all paths to the exit. So if we have determined a set of blocks that overwrite A (and there are no reads in between), we could check if all paths to the exit from A must go through one of the overwriting blocks. I think that matches your suggestion.

Sounds like what I was thinking, yeah.

For objects that are not visible to the caller (e.g. alloca), the intrinsic achieves something slightly different: Given A and B as above, we can eliminate A, if there are no reads of A between A and any of the exits from A. We can stop walking any paths that contain overwrites of A.

I'm confused about why allocas need special treatment. This new set that we're building is essentially a fast way to answer the question "are there potentially unknowable reads after this Def/Phi?" If we know memory (e.g., from an alloca) is unreadable outside of the function, I'd hope we can ignore the set entirely, like this patch is doing now? Baked into my suggestion was the assumption that we were already:

clang-format

Implement conversion for matrix/scalar variants.

Implement conversions for matrix/scalar variants.

Update arithmetic conversion rules after recent discussion on cfe-dev.

In D76611#1955626, @efriedma wrote:

Code looks fine.

I'm a little concerned that the recent clarification in LangRef doesn't actually match the code at the moment, so this could end up exposing an issue with some other transform. (grep for Attribute::SanitizeMemory.)

I assume you are referring to code in other passes, right? I think we probably have code assuming both the new behaviour and the old one and we somehow have to consolidate the code base. Probably best to discuss that at the langref change/llvm-Dev?

Avoid inserting calls by pre-computing set of the last memorydefs or phis before a function exit.

Use placeholder type for incomplete matrix index expressions, as suggested by @rjmccall

Rebased after adding constantrange_including_undef to ValueLattice.

I think it would be helpful to describe what goes wrong in the patch description.

In D76193#1927889, @nikic wrote:

In D76193#1924399, @fhahn wrote:

That's a neat site. It looks very useful. Do you know if there's any more information about the site? Is there a way to also check the patches together with D76228?

With respect to the regression, I am a bit surprised that there are seem to be quite a few assumptions encoded using assume() from clang. I'll check.

If you push a branch that starts with perf/ to your fork, it should get picked up. I've also done a run with the additional patch: Individual commits and all three combined.

Generally, I think this code is a bit convoluted, and I think it would be clearer to rewrite along these lines.

In D76992#1954253, @Ayal wrote:>

Yes, let's go for this cleaner solution. The first phase of VPlan integration using recipes is focused on refactoring ILV logic so that decisions are made and recorded in VPlan rather than during codegen. The latter, taken care of by recipe::execute(), should be as straightforward as possible.

In D76446#1932412, @nikic wrote:

In D76446#1932076, @fhahn wrote:

In D76446#1932013, @nikic wrote:

I'd like to see a unit test that exhaustively tests this for all binary operators (or the ones where it holds). I don't think single element ranges are handled accurately for all the ops (e.g. probably not for urem/srem?)

Yes, ideally we would handle all single element ranges in ConstantRange::binaryOp directly, by delegating to APInt, rather than doing it separately for each one I think. Maybe we should add an APInt::binaryOp?

I don't think that's possible due to layering. APInt is in Support, so it can't depend on IR.

Or just dispatch in ConstantRange::binaryOp?

We probably don't want to make binaryOp() more powerful than the individual methods, so I think your current approach is already fine.

In D73763#1951432, @george.burgess.iv wrote:

Thanks for working on this!

+1 to the general idea of "adding a read-everything intrinsic doesn't smell great," and a further +1 for containing it to only this pass if we do end up needing it.

In D71938#1952589, @uweigand wrote:

I'm now getting an assertion failure in the LNT test suite:
http://lab.llvm.org:8011/builders/clang-s390x-linux-lnt/builds/17826/steps/test-suite/logs/stdio

clang: /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/llvm/llvm/lib/IR/Constants.cpp:1661: static llvm::Constant* llvm::ConstantExpr::getCast(unsigned int, llvm::Constant*, llvm::Type*, bool): Assertion `CastInst::castIsValid(opc, C, Ty) && "Invalid constantexpr cast!"' failed.
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0. Program arguments: /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1.install/bin/clang -DNDEBUG -O3 -DNDEBUG -w -Werror=date-time -w -o CMakeFiles/GCC-C-execute-scal-to-vec1.dir/scal-to-vec1.c.o -c /home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/test/test-suite/SingleSource/Regression/C/gcc-c-torture/execute/scal-to-vec1.c

1. <eof> parser at end of file
2. Per-module optimization passes
3. Running pass 'CallGraph Pass Manager' on module '/home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/test/test-suite/SingleSource/Regression/C/gcc-c-torture/execute/scal-to-vec1.c'.
4. Running pass 'Sparse Conditional Constant Propagation' on function '@main' [...] #9 0x000002aa1a5b6b70 llvm::ConstantExpr::getCast(unsigned int, llvm::Constant*, llvm::Type*, bool) (/home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1.install/bin/clang+0x2ab6b70) #10 0x000002aa19ec233c llvm::ConstantFoldCastOperand(unsigned int, llvm::Constant*, llvm::Type*, llvm::DataLayout const&) (/home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1.install/bin/clang+0x23c233c) #11 0x000002aa1ac69ab0 (anonymous namespace)::SCCPSolver::visitCastInst(llvm::CastInst&) (/home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1.install/bin/clang+0x3169ab0) #12 0x000002aa1ac6ce72 llvm::InstVisitor<(anonymous namespace)::SCCPSolver, void>::visit(llvm::Instruction&) (/home/uweigand/sandbox/buildbot/clang-s390x-linux-lnt/stage1.install/bin/clang+0x316ce72) [...]
   
   that appears to be directly related to this visitCastInst change.

I've moved the expected raw string literals to variables in rG6120cb42f797. It looks like that was enough to silence the warning at least for the llvm-clang-x86_64-expensive-checks-win (http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/22891). Please let me know if there are any other configurations where this did not fix the warning.

In D76992#1951192, @Ayal wrote:

Thanks for following-up quickly. The part that extends VPWidenRecipe to have a User for holding its VPValue operands looks ok, but could the patch make sure that these operands are indeed used correctly by ILV::widenInstruction(), instead of moving the latter over to its execute()? The former may involve refactoring the case that deals with Calls, to record cost-based UseVectorInstrinsic/NeedToScalarize decisions when constructing the VPlan/recipe rather than during codegen; to record InvariantCond of Selects instead of accessing getSCEV(I.getOperand(0)) during codegen, possibly cleaning up to generate ScalarCond OR Cond but not both. In any case, accesses to I’s operands should all change to access User's operands instead, possibly with their underlying ingredient if needed.

I've adjusted the defaults after rG05f0e598ab26: per default, markConstantRange & co assume that the constant range does not contain undef (existing behavior) and isConstantRange/getConstantRange allow constant ranges including undef (existing behavior).

Thanks for taking a look @dexonsmith!

Adjust documentation in a few more places, add assertions to make sure operands of binary ops agree on the layout.

In D76325#1948804, @anemet wrote:

TODO: additional tests.

Is this WIP?

Remove unnecessary updates, use DT::dominates in stead of OrderedInstructions, after DT::dominates now uses the BB local numbering recently committed.

Adjust bb numbers in tests after change to creating BBs first.

Thanks Adam.

Please prefix the commit message with something like [NewGVN]

Thanks Eli!

We should probably also think about what to do with all the tests that contain branches on undef. Also, bugpoint should probably stop creating branches on undef with this spelled out as is?

Thanks for all the input!

Rebased.

Update test checking VPBB iterator, remove newline from printing.

ping :)

In D76327#1944749, @efriedma wrote:

We could make the Module automatically compute a map from intrinsics to overloads of that intrinsic in the module, I guess. Probably not too expensive to maintain.

In D76931#1946483, @aqjune wrote:

Hi,
The semantics of whether br undef is nondet. choice or UB really has been a topic between me and Nuno too.
As @nikic said, we can define it as UB if which branch to take depends on an undef value.

ping

In D76611#1942381, @efriedma wrote:

If I'm understanding correctly, you're saying that SCCP doesn't care about the difference between constantrange and a hypothetical constantrangefromundef, therefore it's fine if the value is actually undef? I guess that's reasonable. But it would be more clear if we represented it explicitly in the lattice.

Update according to comments on cfe-dev.

Thanks for taking a look!