Page MenuHomePhabricator

[AMDGPU] Make overlaps with ScratchRSrcReg fatal errors
AbandonedPublic

Authored by critson on Jun 14 2020, 9:42 PM.

Details

Summary

Overlaps of these will always result in bad code.

Diff Detail

Unit TestsFailed

TimeTest
50 msLLVM.CodeGen/ARM::Unknown Unit Message ("")
Script: -- : 'RUN: at line 2'; /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/build/bin/llc -mtriple=arm-- -run-pass=machine-outliner -verify-machineinstrs /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/llvm/test/CodeGen/ARM/machine-outliner-no-lr-save.mir -o - | /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/build/bin/FileCheck /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang/llvm-project/llvm/test/CodeGen/ARM/machine-outliner-no-lr-save.mir
70 msLLVM.CodeGen/ARM::Unknown Unit Message ("")
Script: -- : 'RUN: at line 2'; c:\ws\prod\llvm-project\build\bin\llc.exe -mtriple=arm-- -run-pass=machine-outliner -verify-machineinstrs C:\ws\prod\llvm-project\llvm\test\CodeGen\ARM\machine-outliner-no-lr-save.mir -o - | c:\ws\prod\llvm-project\build\bin\filecheck.exe C:\ws\prod\llvm-project\llvm\test\CodeGen\ARM\machine-outliner-no-lr-save.mir

Event Timeline

critson created this revision.Jun 14 2020, 9:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 14 2020, 9:42 PM

It seem to cause real test failures.

It seem to cause real test failures.

Currently investigating.
I was, for some reason, naive enough to think this would be fine.
Since the live-in overlap part is new, it'll be that.

arsenm added inline comments.Jun 15 2020, 4:22 AM
llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
331

This should go in the verifier instead

331

There was a patch to add a target verifier hook for function level features, but I'm not sure it was ever committed

critson updated this revision to Diff 270735.Jun 15 2020, 6:26 AM

Remove LiveIn parts.
I have concluded that they are wrong, or at least too complicated for a simple test.
In small functions parts of the ScratchRSrcReg may be formed from registers that
are live-ins, and these will work fine.

arsenm added inline comments.Jun 15 2020, 6:31 AM
llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
339

This is only really an improvement for hand written MIR, and should go in the verifier?

critson marked an inline comment as done.Jun 16 2020, 1:30 AM
critson added inline comments.
llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
339

This can occur at compile time on LLVM IR with an -sgpr-limit set.

arsenm added inline comments.Jun 16 2020, 12:22 PM
llvm/lib/Target/AMDGPU/SIRegisterInfo.cpp
339

Do you mean the attributes to directly limit the number of SGPRs? These should not be used anymore, and this is one of the reasons why.

critson abandoned this revision.Jun 28 2020, 11:52 PM