This is an archive of the discontinued LLVM Phabricator instance.

[TTI] Added TTI pass queries for max load/store-per-memory-intrinsic.
AbandonedPublic

Authored by hussainjk on Nov 11 2019, 9:04 PM.

Details

Summary

Added these query functions to the TargetTransformInfo pass and related support classes:
getMaxStoresPerMemset, getMaxStoresPerMemcpy, getMaxGluedStoresPerMemcpy, getMaxExpandSizeMemcmp, getMaxStoresPerMemmove;
they all wrap a call to the corresponding function in TargetLoweringBase.

I will be using getMaxStoresPerMemcpy and getMaxStoresPerMemset to condition a loop optimization that unrolls a memcpy or memset-patterned loop of unknown but bounded length, up to a maximum threshold on the bound which will be determined by these values. I included the rest for the sake of completion.

Event Timeline

hussainjk created this revision.Nov 11 2019, 9:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 11 2019, 9:04 PM
arsenm added a subscriber: arsenm.Nov 11 2019, 9:27 PM
arsenm added inline comments.
llvm/include/llvm/Analysis/TargetTransformInfo.h
887

Needs an address space

895

Needs source and dest address spaces. The alignment is also a consideration

899–901

I don't understand what I means by glue here. Like SelectionDAG glue? I don't think this is a useful concept to expose to the IR

hussainjk added inline comments.Nov 11 2019, 10:10 PM
llvm/include/llvm/Analysis/TargetTransformInfo.h
887

The underlying function in TargetLoweringBase -- which this wraps -- doesn't take addressing arguments.

895

Likewise.

899–901

AFAIK this refers to groups of stores getting chained together within the memcpy expansion, with there being no guarantees otherwise about consecutive scheduling.

arsenm added inline comments.Dec 21 2019, 3:56 AM
llvm/include/llvm/Analysis/TargetTransformInfo.h
887

That's another problem that needs to be solved

I realize that without any uses of these functions, we can't really add a test case. However, can we at least get a description of why this is useful in the middle end if this patch won't add any users of these functions?

Generally, I am a bit apprehensive about adding functions without callers if we don't have a clear roadmap to when/how these will be used.

I realize that without any uses of these functions, we can't really add a test case. However, can we at least get a description of why this is useful in the middle end if this patch won't add any users of these functions?

Generally, I am a bit apprehensive about adding functions without callers if we don't have a clear roadmap to when/how these will be used.

I have a pending loop optimization where I want to use some of these values (the memset and memcpy ones) for cost modeling. I included the others because they are lumped together, and it seemed like it would be incomplete to only include the two I need.

hussainjk edited the summary of this revision. (Show Details)Jan 22 2020, 8:00 AM

Still needs address space parameters

llvm/include/llvm/Analysis/TargetTransformInfo.h
899–901

This should avoid referring to SelectionDAG specifics

hussainjk marked an inline comment as done.Jan 22 2020, 8:09 AM
hussainjk added inline comments.
llvm/include/llvm/Analysis/TargetTransformInfo.h
887

I consulted with @nemanjai on the best way to resolve this; I will be adding address space arguments with default 0 value to the underlying functions in a separate patch after this gets merged, since it's nontrivial and out of scope for this patch (it involves updating function declarations in several different targets at least). I would then add the address space arguments to these new functions in that patch.

@nemanjai I amended the description with some detail on the work I will be using this in, as discussed. LMK if it should be more detailed.

hussainjk abandoned this revision.Mar 30 2020, 11:13 AM