Oops C++ fail and missed some tests.

There is an existing place in codegen to handle forwarding a store value to the corresponding load - ForwardStoreValueToDirectLoad in lib/CodeGen/SelectionDAG/DAGCombiner.cpp. Handling this in codegen would not require putting the code in multiple places. The code there is relatively simple - it just looks to see if the thing on the chain immediately before the load is the setting store. For the first case in byval-lhs.ll there is a CALLSEQ_START on the chain in between the load and the store. Maybe it is possible to look past that in the chain to see the store. Since the load is for a call there may be a register copy required to replace the load. Where there is a sequence of multiple stores followed by multiple loads it would require looking back in the chain past loads and past stores to fixed stack locations that do not overlap. I don't know if that is allowed, but in theory it could work.

LGTM

LGTM

LGTM

Update diff to show diffs with new tests.

@t.p.northover , any concerns?

Forgot to include the new tests.

LGTM

LGTM

In D109416#3070423, @bmahjour wrote:

In D109416#3067495, @RolandF wrote:

Is this still needed?

Yes, this patch establishes the target interface that is implemented by D109417.

Is this still needed?

LGTM

Test case?

Ping?

Resolve comment.

Resolve some comments.

Improved testing and tightened checks.

LGTM

Update test to use CHECK-COUNT.

Moved the tests to a separate file and added hand-written checks which include the permute control data.

Generated test checks with script.

Add X86 test.

The following file and command should reproduce the failure we are seeing.

This revision causes a traceback when compiling SPEC2017 523.xalanbmk_r with -O3 -m64 -mcpu=power9 -flto and PGO, on the -fprofile-use compile step for XMLDateTime.cpp. Can you please take a look? Let me know if don't have access to SPEC0217 source and need a reproducer. The traceback was as follows:

Update diff to show test changes and respond to comments.

@nemanjai Yes, please, I do need it committed for me. Last time I promise!

Deleted commented out code and added suggested comments.

Added a new TTI method vectorCostAdjustment to consolidate and make uniform for all instruction types the checks and cost modification. Also added direct cost model test.

For memory ops it should be the same as arithmetic. The LSUs are a separate resource from the slices, but a vector load or store still consumes multiple LSUs (2x if aligned, 3x if not). I don't follow why there should be a problem with shuffle - I assume a shuffle will require one or more vector ALU ops.

I don't think adding a new TTI function is necessary, as I think the way I am modeling the costs here is what is expected. The following comment (from TargetTransformInfo.h: getArithmeticInstrCost()) may shed some light:

Fix comment.

In D55461#1326192, @jsji wrote:

I am confused by this patch.

In LoopVectorize.cpp, getInstructionCost returns the execution time cost of an instruction for a given vector.

So I think the cost model here should be related to latency.

Why we need to take into consideration of execution units for execution time cost ?
Considering execution units looks more like throughput cost model?

Nice! Glad to see this stuff get completed.

Address review comments - change variable names, support/test big-endian.

Changed == NULL to == nullptr.

Update to access constant shift amount by dynamic cast and to reverse if from block form to early exit.

Updated to incorporate review comments.

Unlike the php test failure, which is dependent on the library memcmp behaviour and fails for both clang and gcc, the postgres test failure only happens with clang. The uuid regression test fails for clang, and the failure goes away if src/backend/utils/adt/uuid.c is compiled with gcc. The issue is the result for the uuid_internal_cmp function, which is just a 16 byte memcmp. The address of the function is stored in a table of builtins and only called by address, and the complexity of the application and test environment make it difficult to trace back to where this function is called, which may be many places. It might be desirable to just be compatible with gcc. This diff updates the approach to use the gcc-type IPM/SLL/SRA sequence. The sequence is first translated into a SELECT_CMP operation. This makes it easier to perform the memcmp compare to zero optimization (SRA kills the CC). It also should make it easier to add support for LOCHI, since the compare to zero code can be shared, and to get the promotion to 64-bit case with shared code.