This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU]Increased vector length for global/constant loads.
ClosedPublic

Authored by FarhanaAleen on Feb 13 2018, 8:10 PM.

Details

Summary

GCN ISA supports instructions that can read 16 consecutive dwords from memory through the scalar data cache; loadstoreVectorizer should take advantage of the wider vector length and pack 16/8 elements of dwords/quadwords.

Diff Detail

Repository
rL LLVM

Event Timeline

FarhanaAleen created this revision.Feb 13 2018, 8:10 PM
t-tye added inline comments.Feb 13 2018, 8:46 PM
lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp
262 ↗(On Diff #134126)

Does amdgpu only support gfx6 (si) and above? I thought northern islands was supported by the r600 backend.

lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h
121 ↗(On Diff #134126)

I did not see where in this patch these new functions are being used.

Does amdgpu only support gfx6 (si) and above? I thought northern islands was supported by the r600 backend.

AMDGPU contains the general target information for all gpus and specific target implementation is supported by the specific backend; such as r600 and SI backends.

lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h
121 ↗(On Diff #134126)

These are not new functions; the default definitions can be found in TargetTransformInfoImpl.h. loadStoreVectorizer was using the default implementation of these functions for AMDGPU since there were no implementation of these functions for AMDGPU.
I have copy-pasted the default definitions here for convenience.

unsigned getLoadVectorFactor(unsigned VF, unsigned LoadSize,

                             unsigned ChainSizeInBytes,
                             VectorType *VecTy) const {
  return VF;
}

unsigned getStoreVectorFactor(unsigned VF, unsigned StoreSize,
                              unsigned ChainSizeInBytes,
                              VectorType *VecTy) const {
  return VF;
}

Please fix the tests, otherwise looks good.

test/CodeGen/AMDGPU/load-constant-f32.ll
12 ↗(On Diff #134126)

Run tests through opt -instnamer. There should be no numeric values.

Renamed the instructions to get rid of the numeric values.

This revision is now accepted and ready to land.Feb 16 2018, 3:03 PM
This revision was automatically updated to reflect the committed changes.