This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Add CPSR as an implicit use of t2IT
ClosedPublic

Authored by samparker on Feb 26 2020, 8:24 AM.

Details

Summary

This use is already attached to the BUNDLE instruction but is lost after finalisation, which makes tracking use-def difficult after that.

Diff Detail

Event Timeline

samparker created this revision.Feb 26 2020, 8:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 26 2020, 8:24 AM

I would agree that IT uses CPSR.

llvm/test/CodeGen/Thumb2/LowOverheadLoops/it-block-mov.mir
40

Is this intended?

samparker marked an inline comment as done.Feb 27 2020, 1:19 AM
samparker added inline comments.
llvm/test/CodeGen/Thumb2/LowOverheadLoops/it-block-mov.mir
40

Bah - no! cheers!

samparker updated this revision to Diff 246890.EditedFeb 27 2020, 1:33 AM

Removed large change from test file.

dmgreen accepted this revision.Feb 27 2020, 1:46 AM

LGTM

llvm/test/CodeGen/Thumb2/LowOverheadLoops/it-block-mov.mir
39–40

The check lines still don't have the CPSR. Maybe regenerate before committing to keep the checks up-to-date.

This revision is now accepted and ready to land.Feb 27 2020, 1:46 AM

This doesn't look correct to me - the IT instruction writes to CPSR (which we represent as ITSTATE), but it doesn't read anything from it.

The reference manual, at least the v7m anyway, states 'IT does not affect the condition code flags', and I believe the backend only considers the cpsr for condition codes. I understood that the itstate is then representing the masking that the IT provides.

@olista01 After our discussion, I will revert this after D75185 and D75245 are committed, as these should fix the issue properly and save me from making loads of test changes again.

This revision was automatically updated to reflect the committed changes.