Page MenuHomePhabricator

[PowerPC] Change target data layout for 16-byte stack alignment
ClosedPublic

Authored by saghir on Feb 8 2021, 7:31 AM.

Details

Summary

This changes the target data layout to make stack align to 16 bytes
on Power10. Before this change, stack was being aligned to 32 bytes.

Diff Detail

Event Timeline

saghir created this revision.Feb 8 2021, 7:31 AM
saghir requested review of this revision.Feb 8 2021, 7:31 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 8 2021, 7:31 AM
saghir added reviewers: Restricted Project, nemanjai, jsji.Feb 8 2021, 7:32 AM
nemanjai requested changes to this revision.Feb 11 2021, 7:09 AM

Can you please merge the tests into one file. There is no compelling reason to split them up and it is more difficult to review and make sense of what is going on. The test case should have:

  1. A function that allocates a 32-byte vector (i.e. defined with __attribute__((vector_size(32))))
  2. A function that allocates a 32-byte aligned vector (i.e. defined with __attribute__((aligned(32))))
  3. A function that allocates an array that wouldn't be aligned to something in excess of stack alignment but gets over-aligned due to vectorization

You can achieve 3. above with something like this:

$ cat t.c
char Arr1[64];
void test(short *);
void cpy() {
  short Arr2[64];
  for (int i = 0; i < 64; i++)
    Arr2[i] = Arr1[i];
  test(Arr2);
}

clang -O3 -S t.c -emit-llvm -Xclang -disable-llvm-passes

Then the test case should run something like the following:
opt --passes=sroa,loop-vectorize,loop-unroll,instcombine t.ll -S -o t.opt.ll -vectorizer-maximize-bandwidth --mtriple=powerpc64le--
And check the alignment of the alloca and vector store. They should be 16 and not 32 even though the vector is 32 bytes wide.

This revision now requires changes to proceed.Feb 11 2021, 7:09 AM
saghir updated this revision to Diff 323307.Feb 12 2021, 6:20 AM

Merged tests into one file.

saghir updated this revision to Diff 323451.Feb 12 2021, 12:39 PM

Updated tests to add run line for llc and target datalayout.

nemanjai accepted this revision.Feb 12 2021, 12:54 PM

Thank you for your patience. LGTM now.

This revision is now accepted and ready to land.Feb 12 2021, 12:54 PM
This revision was landed with ongoing or failed builds.Mar 8 2021, 6:13 AM
This revision was automatically updated to reflect the committed changes.