It seems to me Cost::RateFormula() form LSR is the only user that can be affected by the change. Would you please double-look the use case?

Fixed SLOW/FAST misplacement in test

Updated test as requested.

Needs a test.

Ping...

Thanks!

In D110100#3014761, @RKSimon wrote:

In D110100#3014729, @samparker wrote:

Do you think we should stop using default cost kinds entirely?

I think this would be for the best, and I thought I already changed most of the calls to be explicit.

@dfukalov Is this something you'd be willing to do in this patch? At least for getInstructionCost + getInterleavedMemoryOpCost (TTI + Impl)?

Fixed all clang-tidy warnings except "invalid case style".

Addressed comment.

Fixed clang-tidy errors.

In D108596#2962039, @foad wrote:

No objections from me, seems like a good cleanup. Do you plan to do more of this? What's the end goal?

Yes, I'm going to try additional NFC split out of parts like R600DAGToDAGISel, R600MCInstLower etc.

Removed no new line messages.

Thanks!

In D107898#2956403, @smeenai wrote:

This is breaking our cross-compilation from Linux to Windows. It doesn't seem like the CMake Linux platform files (at least on 3.20.3) define these variables explicitly (I guess they're just relying on them being implicitly empty), so we still see .exe as our CMAKE_EXECUTABLE_SUFFIX, which breaks running tblgen.

Not sure what the best solution is for that.

Can you please provide cmake config parameters (and possible toolchain description) to reproduce the issue?

In D108285#2952449, @arsenm wrote:

Conflicts with my D108285 stack

Did you mean the stack based on D108158?

Fix typo.

Using introduced LLVM_HOST_EXECUTABLE_SUFFIX variable to add the extension for native binary.

In D107898#2941701, @beanz wrote:

Here's a little terrible blurb of CMake that should actually be a very portable solution to this:
...
If you add this blurb into llvm's config-ix.cmake, you can use LLVM_HOST_EXECUTABLE_SUFFIX the same way you just updated the patch to use CMAKE_EXECUTABLE_SUFFIX, and it should be portable for cross-compilation too.

Using CMAKE_EXECUTABLE_SUFFIX, removed condition.

AMDGPU tests LGTM, thanks!

Changes in AMDGPU tests LGTM, thanks!

Yes, using InstructionCost as number of parts may be confusing, but

1. The function is also used in line 7245 of LoopVectorize.cpp so "Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll" test starts to assert in @test_no_sve just after getTypeLegalizationCost() returns invalid cost.
2. The same logic was used for getRegUsageForType() in D102541 for number of registers so I decided to repeat it here.

Do not call expectedCost(VF) second time just for assert.

In D102358#2789473, @nikic wrote:

It's already approved? ^^

Ooops, sorry for pinging.

Rebased, ping.

Gentle ping...

@sdesmalen this is needed to return from getTypeLegalizationCost() invalid costs, so at some point invalid legalization should be processed to a reg usage value, instead of asserting in Optional<>::getValue().
What do you think, should it be processed explicitly in LV or yet in BasicTTIImplBase<>::getRegUsageForType?
In latter case, what reg usage value should it return for invalid (e.g. scalable vectors) legalization cost?

Yes, I thought about packing offset to MemDepResult, but it is already a packed pointer with other info and it seems to be overkill to double its size for a quite rare offset value.

getDependency() call is just a start of processing in GVN, and MemoryDependenceResults contains a number of a load' dependencies. One of them was NonLocal clobbering dependency between %tmp4.1 = load i8... and %tmp6.1 = load atomic i8... since latter is atomic (e.g. MemoryDependenceAnalysis.cpp$494-500). And GVN got this clobbering dependency not from getDependency().