This is an archive of the discontinued LLVM Phabricator instance.

[Xtensa] Codegen support for memory operations
Needs ReviewPublic

Authored by andreisfr on Mar 9 2023, 12:13 AM.

Details

Summary

Implement codegen for address operands of the load operations
L8UI/L16UI/L16SI/L32I and store operations S8I/S16I/S32I.

Diff Detail

Event Timeline

andreisfr created this revision.Mar 9 2023, 12:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2023, 12:13 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
andreisfr requested review of this revision.Mar 9 2023, 12:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2023, 12:13 AM
myhsu added a reviewer: myhsu.Mar 9 2023, 9:05 AM
andreisfr updated this revision to Diff 508576.Mar 27 2023, 4:43 AM

Minor code formatting.

myhsu added a comment.Mar 27 2023, 8:45 AM

I just realized that this patch along with nearly all other patches in the series don't have tests, what's your plan on that?
Also, it might be a good idea to add more reviewers, potentially backend maintainers.

andreisfr edited the summary of this revision. (Show Details)Mar 27 2023, 3:16 PM

Hi @myhsu, thank you for your comment. We have plans to add also some tests to the current functionality.

aykevl added a subscriber: aykevl.Mar 27 2023, 6:50 PM

@andreisfr I believe every patch should stand on its own and have tests for its own functionality. Even if the backend won't be fully functional without all parts included, each commit should still have the ability to be tested as it is added.

JKRhb added a subscriber: JKRhb.Mar 30 2023, 12:55 PM
andreisfr updated this revision to Diff 544644.Jul 27 2023, 1:10 AM

Added minimal support for stack operations like storeRegToStackSlot/loadRegFromStackSlot/eliminateFrameIndex functions to make possible implement different tests. Added test for load/store operations on stack.

arsenm added a comment.Aug 1 2023, 5:42 PM

Basically lgtm with some nits

llvm/lib/Target/Xtensa/XtensaISelDAGToDAG.cpp
55

s/is not/are not/

Can you make this a DiagnosticInfoUnsupported instead?

67–81

Move this to a separate util function

llvm/lib/Target/Xtensa/XtensaRegisterInfo.cpp
67–68

define and init on same line?

80

init on definition

86–100

Move to separate util function

arsenm requested changes to this revision.Aug 23 2023, 4:46 PM
arsenm added inline comments.
llvm/test/CodeGen/Xtensa/stack-access.ll
21–22

weird indent

This revision now requires changes to proceed.Aug 23 2023, 4:46 PM
andreisfr updated this revision to Diff 558215.Thu, Dec 7, 12:11 AM

Updated according latest changes in main branch and reviewers comments.