ping again:)

ping

In D140260#4004726, @lebedev.ri wrote:

In D140260#4004038, @mdchen wrote:

In D140260#4003727, @lebedev.ri wrote:

It think the loop-invariantness precondition for these is overly restrictive.

Any suggestion on that?

For context. loading i64 and loading <8 x i8> and bitcasting to i64 should be the same thing.

Let me try to catch your point, the key difference here is loading <8 x i8> cannot appear in a scalar loop and the legality check will ensure that. Thus the vector operand of bitcast must be defined outside the loop.

In D140260#4003727, @lebedev.ri wrote:

It think the loop-invariantness precondition for these is overly restrictive.

Test checks will be updated later.

that seems like a genuinely useful feature that would be blocked by treating the test as valid. This code is just an extreme example of that transformation really (where we can reduce the allocation to 0, presumably because past optimizations have removed all significant uses).

Thanks for your reply!

There's this line in the semantics for llvm.fma:

When specified with the fast-math-flag ‘afn’, the result may be approximated using a less accurate calculation.

My reading is the underlying operation must be maintained (i.e. a fused multiply-add)

@RosieSumpter gentle ping: )

@RosieSumpter Thanks for the patch! I'm in question that whether llvm.fma.* should also be considered a valid candidate here. LangRef describes that

If a fused multiply-add is required, the corresponding llvm.fma intrinsic function should be used instead.

In D95142#3157371, @jrtc27 wrote:

In D95142#3157370, @mdchen wrote:

The pointee type is meaningless in the IR and will be going away.

@arsenm @arichardson Hi, this change let user-defined string functions(e.g., size_t strlen(int*)) pass the check and considered a libfunc, which can cause failure in later optimizations like optimizeStrLen which assumes CharSize is 8. And pointed type does matter in this case. What do you think?

The code should've been compiled with either -fno-builtin-strlen or -ffreestanding, or used the right type for strlen, anything else is invalid. See -Wincompatible-library-redeclaration.

The pointee type is meaningless in the IR and will be going away.

@paulwalker-arm Thanks. Would you please take another look if the test looks ok to you now?

Update test.

@lebedev.ri I think the problem is when %0 is negative, the original code gives a definite result while the transformed IR is UB.

Rebase.

Update the comments in invariant-store-vectorization-2.ll.

Address @fhahn 's suggestions.

Correct invariant-store-vectorization-2.ll check lines(the previous version is generated with a version with D104148 code).

Ops... invariant-store-vectorization-2.ll is not in the right status, will update it soon.

Address @fhahn's comments.

Hi @fhahn , to address your comments in D104148, more tests need to be updated here, please take another look, thanks very much!

Updated.

Updated.

Updated the tests. This needs to be merged after D105438.

@fhahn Thanks for your advice. I modified these related tests in D105438. After that is merged, I'll update this revision with one looks cleaner.

ping : )

Ping

Updated previous failed x86 cases.

Added a test case.

LGTM, but please wait for a couple of days for experts in this area to review: )

@lebedev.ri

It fails even if i bump those limits to 0.1M iterations.
This is clearly an infinite combine cycle.

If that's the case, isn't it more reasonable to replace the fatal_error with assert so that only developers sense that?

In D99481#2790122, @aqjune wrote:

Reduced .ll:

reduce.ll3 KBDownload

$ opt -instcombine -S -o - reduce.ll
LLVM ERROR: Instruction Combining seems stuck in an infinite loop after 100 iterations.
...

In D102594#2766167, @tejohnson wrote:

lgtm. this seems fine to me, but presumably we would have a compile error if there was ever a conflict in practice with one of these files?

Did a fully scan and modified more files that have the same issue.

In D102594#2762526, @ChuanqiXu wrote:

@david-arm Glad to know that. Thank you for the work!

@david-arm Hi David, there's a tiny issue of the commit that the llvm.experimental.stepvector intrinsic is not well indexed in the changes to Langref.rst, and now it resides under llvm.experimental.vector.splice. I guess it's not intended and leave a message here FYI:)

ping

@mkazantsev Could you commit this for me, thanks!
My commit access application has been approved so I'll commit by myself:)

ping

Add a test case.

@lebedev.ri Thanks, will add a test. Besides, the bug fixed in ea7ab5a42cd4 seems to have the same root cause.

@kuhar @lebedev.ri I'm lucky to have a test case which requires exactly 1001 times iteration thus aborts. And would it be better just break out rather than report a fatal error?

In D87879#2384112, @fhahn wrote:

Actually, could we get similar problems for dependencies other than output, eg if we have a conditional load that is later used?

Yes, there're similar cases our team have found related to data dependence that may prohibit interchange. And I'd like to spend more time to see if there's a sophisticated solution.

ping

@fhahn Thanks for the suggestions, please take another look.

add a test case