Page MenuHomePhabricator

Make fixed-abi default for AMD HSA OS

Authored by madhur13490 on Feb 9 2021, 6:50 AM.



fixed-abi uses pre-defined and predictable
SGPR/VGPRs for passing arguments. This patch makes
this scheme default when HSA OS is specified in triple.

Diff Detail

Unit TestsFailed

50 msx64 debian > Polly.ScopInfo::user_provided_assumptions.ll
Script: -- : 'RUN: at line 1'; opt -polly-process-unprofitable -polly-remarks-minimal -polly-use-llvm-names -polly-import-jscop-dir=/mnt/disks/ssd0/agent/llvm-project/polly/test/ScopInfo -polly-codegen-verify -pass-remarks-analysis="polly-scops" -polly-scops -disable-output < /mnt/disks/ssd0/agent/llvm-project/polly/test/ScopInfo/user_provided_assumptions.ll 2>&1 | FileCheck /mnt/disks/ssd0/agent/llvm-project/polly/test/ScopInfo/user_provided_assumptions.ll

Event Timeline

madhur13490 created this revision.Feb 9 2021, 6:50 AM
madhur13490 requested review of this revision.Feb 9 2021, 6:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2021, 6:50 AM
arsenm added inline comments.Feb 9 2021, 8:51 AM

This isn't just changing the default, this is breaking the flag. You should check if the option wasn't specified

rebase, address Matt's comments

arsenm added inline comments.Feb 10 2021, 6:29 AM
2 ↗(On Diff #322623)

You should keep the two checks with the flag on and off

Keep both flags

madhur13490 marked 2 inline comments as done.Feb 10 2021, 8:16 AM
arsenm added inline comments.Feb 11 2021, 3:25 PM

This test should force use the variable ABI. We already have a dedicated test for the fixed ABI for these inputs (callee-special-input-sgprs-fixed-abi.ll)

force varabi on callee-special-input-sgprs.ll

arsenm accepted this revision.Feb 17 2021, 11:29 AM
This revision is now accepted and ready to land.Feb 17 2021, 11:29 AM
This revision was landed with ongoing or failed builds.Feb 19 2021, 7:05 AM
This revision was automatically updated to reflect the committed changes.

Setting this 'true' on everything is expected to be a regression. Specifically any code that uses functions, but doesn't use indirect functions, is expected to be slower than it was before. Suggest reverting this patch.

Alternative would look something like:

  • if there is any indirect call, set this true
  • otherwise, leave it unchanged

That will fix things using indirect calls without regressing everything else.

note: llvm::Function::hasAddressTaken() looks useful for that