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 > LLVM.CodeGen/MIR/AMDGPU::machine-function-info-no-ir.mir
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/llc -mtriple=amdgcn-amd-amdhsa -run-pass=none -verify-machineinstrs /mnt/disks/ssd0/agent/llvm-project/llvm/test/CodeGen/MIR/AMDGPU/machine-function-info-no-ir.mir -o - | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck -check-prefixes=FULL,ALL /mnt/disks/ssd0/agent/llvm-project/llvm/test/CodeGen/MIR/AMDGPU/machine-function-info-no-ir.mir
80 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
80 msx64 windows > LLVM.CodeGen/MIR/AMDGPU::machine-function-info-no-ir.mir
Script: -- : 'RUN: at line 1'; c:\ws\w16n2-1\llvm-project\premerge-checks\build\bin\llc.exe -mtriple=amdgcn-amd-amdhsa -run-pass=none -verify-machineinstrs C:\ws\w16n2-1\llvm-project\premerge-checks\llvm\test\CodeGen\MIR\AMDGPU\machine-function-info-no-ir.mir -o - | c:\ws\w16n2-1\llvm-project\premerge-checks\build\bin\filecheck.exe -check-prefixes=FULL,ALL C:\ws\w16n2-1\llvm-project\premerge-checks\llvm\test\CodeGen\MIR\AMDGPU\machine-function-info-no-ir.mir

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

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