Refactor EventFactory, Compilers and Parsers by xeren · Pull Request #1009 · hernanponcedeleon/Dat3M

xeren · 2026-03-13T11:50:15Z

Mostly cleans up some imports, but also

outlines some memory barrier generations in compilers.
outlines uses of ProgramBuilder.addChild in litmus parsers.
integrates FenceNameRepository into EventFactory

Modifications to `EventFactory`

Added

AArch64.newRMWLoadExclusive, AArch64.newRMWStoreExclusive
X86.newLoadFence, newStoreFence (still not implemented, but merges two places)
RISCV.newRMWLoadExclusive
Power.newRMWLoadExclusive

Modified

newRMWStoreExclusive, newRMWStoreExclusiveWithMo
AArch64.newDmbBarrier, etc.
AArch64.newBarrier(String,String) (moved from newFenceOpt)
RISCV.newRMWStoreConditional
newFakeCtrlDep(Register) (returns List<Event> and does no longer accept a Label)

hernanponcedeleon · 2026-03-23T14:48:27Z

@xeren can you please rebase and fix the conflicts?

…actory # Conflicts: # dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/VisitorLitmusAArch64.java # dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/VisitorLitmusC.java # dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/VisitorLitmusPPC.java # dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/VisitorLitmusPTX.java # dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/VisitorLitmusRISCV.java # dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/VisitorLitmusVulkan.java # dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/VisitorLitmusX86.java # dartagnan/src/test/java/com/dat3m/dartagnan/others/miscellaneous/AnalysisTest.java

hernanponcedeleon

Why are you adding newRMWLoadExclusive for each arch if they all end up calling the general case? While I could see an argument in the factory for having the method for each arch, we do have other "common" events that use a single method, e.g. newRMWStoreExclusiveWithMo

dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorArm8.java

ThomasHaas · 2026-03-26T15:03:39Z

Why are you adding newRMWLoadExclusive for each arch if they all end up calling the general case? While I could see an argument in the factory for having the method for each arch, we do have other "common" events that use a single method, e.g. newRMWStoreExclusiveWithMo

I agree with you but maybe from a different point of view. I think it becomes more and more unclear of whether a XYZ.newEvent factory method generates a core event or not. For parsers, you want to NOT generate core events to support compilation(*), but for the Compilation passes you need to ensure to generate only core events.
I think this is particularly true for the C11.newThreadFence events which should NOT be called by the parser (for the most part) and only during Compilation. And those can reasonably well be moved in the dedicated compilation pass.
For example, the LKMM compiler has some helper methods for the lowering that are not otherwise exposed to parsers/other callers.

(*) It is fine to generate core events from the parser, but not if you intend to support compilation to other targets on those events (at least in our current architecture).

Fix missing tag.

xeren · 2026-03-26T18:52:28Z

With the different newRMWLoadExclusive methods, I wanted to group all the usages in the program target-wise. It should have made them easier to be altered per-architecture. Ideally, Common, newRMWLoadExclusive, newLoadWithMo, newStoreWithMo and newFence are never used directly by parsers. As you noticed, my refactoring was incomplete.

dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/VisitorLitmusC.java

dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/VisitorLitmusPPC.java

dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/VisitorLitmusPTX.java

dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/VisitorLitmusRISCV.java

dartagnan/src/main/java/com/dat3m/dartagnan/program/event/EventFactory.java

dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorArm8.java

dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorPower.java

dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorRISCV.java

Remove EventFactory.C11 Remove EventFactory.Linux.newFence(String) Rename EventFactory.Spirv methods Rename EventFactory.Linux methods

Outline newFakeCtrlDependency(Register) Outline newAssignExecutionStatus(Register, Event)

xeren · 2026-04-08T18:01:48Z

In the latest commit I removed the individual barrier constructors in EventFactory, like AArch64.newDmbIshBarrier(). There is no use in having them around. The only users were the respective compilers, where I added individual methods in earlier commits.

From my side, this PR should be ready to merge.

dartagnan/src/main/java/com/dat3m/dartagnan/program/event/EventFactory.java

dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/VisitorLitmusRISCV.java

dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorC11.java

dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorRISCV.java

ThomasHaas · 2026-04-10T15:00:43Z

Looks good to me. @hernanponcedeleon might want to take look once he has time.

dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/visitors/VisitorLitmusX86.java

dartagnan/src/main/java/com/dat3m/dartagnan/parsers/program/ProgramParser.java

hernanponcedeleon · 2026-04-11T19:42:50Z

dartagnan/src/main/java/com/dat3m/dartagnan/program/event/EventFactory.java

+        public static Event newStoreOp(Expression address, IntBinaryOp operator, Expression operand, MemoryOrder mo) {
+            final Event st = EventFactory.Common.newRmwOp(address, operator, operand);
+            // ignore acquire tag
+            st.addTags(toRelTag(mo));
+            st.setMetadata(STOP_PRINTING);
+            return st;
+        }


I am confused about this method and newLoadOp. The names suggest they are simply load or store operations, but then we call RMW methods. Also, the load one adds both release and acquire tags, but the store one ignores the acquire one. Why?

Those are LDADD/STADD (and its variants, i.e, LD<OP> and ST<OP>) in arm8. That's also why the StoreOp ignores acquire.
He just moved the code from the Arm8Visitor into the EventFactory.

But then why does not the load one ignore the release?

Maybe you forgot what these operations mean :)
LD<OP> == fetch_op, i.e., it has a proper load followed by a proper store both of which can have memory orderings.
ST<OP> == store_op (atomic reduction), i.e., it technically has no load (non-returning) and only its store can have a memory ordering. To be fair, the code should probably throw an exception when trying to use ACQ/ACQ_REL memory orderings for ST<OP>.

Yes, like with newBarrier/newFence, newLoadReserved/newLoadExclusive, etc.. I chose these names to correspond with the instructions of the associated ISAs/Languages (and to abstract from our IR's).

To be fair, the code should probably throw an exception when trying to use ACQ/ACQ_REL memory orderings for ST<OP>.

This is still open

dartagnan/src/main/java/com/dat3m/dartagnan/program/event/EventFactory.java

xeren added 2 commits March 13, 2026 11:18

Refactor

595744e

fixup! Refactor

62a9f58

hernanponcedeleon reviewed Mar 25, 2026

View reviewed changes

dartagnan/src/main/java/com/dat3m/dartagnan/program/processing/compilation/VisitorArm8.java Outdated Show resolved Hide resolved

xeren added 4 commits March 26, 2026 18:35

fixup! Refactor

952ad54

Fix missing tag.

Move newRMWLoadExclusive from EventFactory to compilers

b6d6ab2

fixup! Move newRMWLoadExclusive from EventFactory to compilers

0385200

fixup! Move newRMWLoadExclusive from EventFactory to compilers

ebd62db

xeren added 2 commits March 26, 2026 22:34

Refactor VisitorLitmusAArch64

c3acefb

fixup! Refactor VisitorLitmusAArch64

b82a538

ThomasHaas reviewed Apr 2, 2026

View reviewed changes

xeren added 12 commits April 7, 2026 17:01

Compiled RISCV LXSX pairs always require matching addresses.

b71cdd6

Outline arch-specific EventFactory usages

717047f

Implement feedback

87fbf2c

Remove EventFactory.C11 Remove EventFactory.Linux.newFence(String) Rename EventFactory.Spirv methods Rename EventFactory.Linux methods

fixup! Outline arch-specific EventFactory usages

ed5d0fd

fixup! Outline arch-specific EventFactory usages

094c596

fixup! Implement feedback

d373dec

fixup! Implement feedback

f127bc7

Refactor compilers

0ad394a

Outline newFakeCtrlDependency(Register) Outline newAssignExecutionStatus(Register, Event)

Modify EventFactory.newFakeCtrlDep(Register)

4a1b125

Merge branch 'development' into refactorEventFactory

0acea3f

Merge barrier constructors in EventFactory

1b6bfac

fixup! Merge barrier constructors in EventFactory

be23c27

ThomasHaas reviewed Apr 8, 2026

View reviewed changes

dartagnan/src/main/java/com/dat3m/dartagnan/program/event/EventFactory.java Show resolved Hide resolved

xeren added 2 commits April 9, 2026 16:12

Wrap all exceptions from ProgramParser in ParsingException

ec60666

fixup! Wrap all exceptions from ProgramParser in ParsingException

b513bfc

ThomasHaas reviewed Apr 9, 2026

View reviewed changes

xeren added 2 commits April 10, 2026 14:30

Renaming

84d49dc

Use AMO in C11 and IMM, Renaming

ac5e215

Revert AMO in IMM

4d8aaa9

hernanponcedeleon reviewed Apr 11, 2026

View reviewed changes

xeren added 3 commits April 13, 2026 11:44

Implement feedback

99d7da2

Check memory order in AArch64's STADD etc.

562b3eb

fixup! Check memory order in AArch64's STADD etc.

dd8d183

Conversation

xeren commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Modifications to EventFactory

Added

Modified

Uh oh!

hernanponcedeleon commented Mar 23, 2026

Uh oh!

hernanponcedeleon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ThomasHaas commented Mar 26, 2026

Uh oh!

xeren commented Mar 26, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xeren commented Apr 8, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ThomasHaas commented Apr 10, 2026

Uh oh!

Uh oh!

Uh oh!

hernanponcedeleon Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

ThomasHaas Apr 11, 2026

Choose a reason for hiding this comment

Uh oh!

hernanponcedeleon Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

ThomasHaas Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xeren Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

hernanponcedeleon Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

xeren commented Mar 13, 2026 •

edited

Loading

Modifications to `EventFactory`

ThomasHaas Apr 12, 2026 •

edited

Loading