This is an archive of the discontinued LLVM Phabricator instance.

Add support for intrinsic llvm.ppc.eieio
ClosedPublic

Authored by anil9 on Oct 16 2019, 12:59 PM.

Details

Summary

Add support for the intrinsic llvm.ppc.eieio to emit the instruction eieio.

Diff Detail

Event Timeline

anil9 created this revision.Oct 16 2019, 12:59 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 16 2019, 12:59 PM
lei added a subscriber: lei.Oct 22 2019, 11:28 AM
lei added inline comments.
llvm/lib/Target/PowerPC/PPCInstrInfo.td
2287

This used to be under PowerPC Instructions used for assembler/disassembler only. Is this no longer true? You seem to have moved this into PPC32 Store Instructions section. What does this intrinsic do?

We generally don't move unrelated code around. Maybe keep this where it was and add your new def under PowerPC Assembler Instruction Aliases?

anil9 added inline comments.Oct 31 2019, 7:53 AM
llvm/lib/Target/PowerPC/PPCInstrInfo.td
2287

We are exposing it as an intrinsic, so I think it is not just for assembler/disassembler only so should not be in the previous place anymore.

It is a form of barrier that acts on only load and stores related to data not instructions.

steven.zhang added inline comments.
llvm/lib/Target/PowerPC/PPCInstrInfo.td
2284

Please set the has side effect bit to make it clear.

anil9 marked an inline comment as done.Jan 7 2020, 11:19 AM
anil9 added inline comments.
llvm/lib/Target/PowerPC/PPCInstrInfo.td
2284

The purpose of this patch is to expose this instruction using a built in function. You are correct that using the hasSideEffects bit will make it clear, but I checked that there are a lot of synchronization instructions without the hasSideEffects bit set, I think all of them should be handled together in a separate patch.

amyk added a subscriber: amyk.Jan 9 2020, 8:44 PM
amyk added inline comments.
llvm/lib/Target/PowerPC/PPCInstrInfo.td
2286

I think this indenting is a bit off -- align the second line starting from the double quote under the 3 in the top line.

anil9 updated this revision to Diff 237404.Jan 10 2020, 11:44 AM

Addressed review comment. [NFC]

anil9 marked 4 inline comments as done.Jan 10 2020, 11:45 AM
nemanjai requested changes to this revision.Jan 12 2020, 10:06 AM

The code looks fine to me, but where is the test case?

This revision now requires changes to proceed.Jan 12 2020, 10:06 AM
anil9 updated this revision to Diff 239993.Jan 23 2020, 1:24 PM

Added test case.

nemanjai accepted this revision.Jan 27 2020, 4:53 AM

LGTM.

This revision is now accepted and ready to land.Jan 27 2020, 4:53 AM
This revision was automatically updated to reflect the committed changes.