This is an archive of the discontinued LLVM Phabricator instance.

Differential D120665

[AArch64] Fix the TuneExynosM4 entry in lib/Target/AArch64/AArch64.td
ClosedPublic

Authored by david-arm on Feb 28 2022, 9:12 AM.

Details

Reviewers
sdesmalen
dmgreen
tmatheson
Commits
rGf9331c9a2c87: [AArch64] Fix the TuneExynosM4 entry in lib/Target/AArch64/AArch64.td
Summary

A bug was introduced in 5ea35791e6642f6160819c930f79b0f57ad0e907 that
gave the wrong name and description for TuneExynosM4. This patch fixes
that and changes it back to m3.

Diff Detail

Unit TestsFailed

TimeTest
60,040 msx64 debian > libFuzzer.libFuzzer::large.test

Event Timeline

david-arm created this revision.Feb 28 2022, 9:12 AM
Herald added subscribers: hiraditya, kristof.beyls. · View Herald TranscriptFeb 28 2022, 9:12 AM
david-arm requested review of this revision.Feb 28 2022, 9:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 28 2022, 9:12 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
sdesmalen added inline comments.Feb 28 2022, 9:17 AM
llvm/lib/Target/AArch64/AArch64.td
821–823

should this be ExynosM4 as well then?

Harbormaster completed remote builds in B151763: Diff 411820.Feb 28 2022, 9:57 AM
dmgreen added inline comments.Feb 28 2022, 11:39 AM
llvm/lib/Target/AArch64/AArch64.td
821–823

It wasn't in the original code. They shared the ExynosM3 ProcFamily. Adding it doesn't add a huge amount unless there is some reason to distinguish the two.

sdesmalen added inline comments.Mar 1 2022, 3:21 AM
llvm/lib/Target/AArch64/AArch64.td
821–823

Makes sense. Perhaps a comment would be useful in that case, e.g. /*Re-uses ExynosM3 proc family*/ or something like that.

david-arm updated this revision to Diff 412721.Mar 3 2022, 7:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2022, 7:40 AM
david-arm marked 3 inline comments as done.Mar 3 2022, 7:41 AM
tmatheson accepted this revision.Mar 3 2022, 7:41 AM
This revision is now accepted and ready to land.Mar 3 2022, 7:41 AM
Harbormaster completed remote builds in B152377: Diff 412721.Mar 3 2022, 8:07 AM
This revision was landed with ongoing or failed builds.Mar 4 2022, 2:27 AM
Closed by commit rGf9331c9a2c87: [AArch64] Fix the TuneExynosM4 entry in lib/Target/AArch64/AArch64.td (authored by david-arm). · Explain Why
This revision was automatically updated to reflect the committed changes.
david-arm added a commit: rGf9331c9a2c87: [AArch64] Fix the TuneExynosM4 entry in lib/Target/AArch64/AArch64.td.

Revision Contents

PathSize
llvm/
lib/
Target/
AArch64/
5 lines

Diff 412721

llvm/lib/Target/AArch64/AArch64.td

Show First 20 Lines • Show All 812 Lines • ▼ Show 20 Linesdef TuneExynosM3 : SubtargetFeature<"exynosm3", "ARMProcFamily", "ExynosM3",
FeatureFuseAddress, FeatureFuseAddress,
FeatureFuseAES, FeatureFuseAES,
FeatureFuseCCSelect, FeatureFuseCCSelect,
FeatureFuseLiterals, FeatureFuseLiterals,
FeatureLSLFast, FeatureLSLFast,
FeaturePostRAScheduler, FeaturePostRAScheduler,
FeaturePredictableSelectIsExpensive]>; FeaturePredictableSelectIsExpensive]>;
def TuneExynosM4 : SubtargetFeature<"exynosm3", "ARMProcFamily", "ExynosM3", // Re-uses some scheduling and tunings from the ExynosM3 proc family.
"Samsung Exynos-M3 processors", def TuneExynosM4 : SubtargetFeature<"exynosm4", "ARMProcFamily", "ExynosM3",
"Samsung Exynos-M4 processors",
sdesmalenUnsubmitted
Done
ReplyInline Actions

should this be ExynosM4 as well then?

sdesmalen: should this be ExynosM4 as well then?
dmgreenUnsubmitted
Done
ReplyInline Actions

It wasn't in the original code. They shared the ExynosM3 ProcFamily. Adding it doesn't add a huge amount unless there is some reason to distinguish the two.

dmgreen: It wasn't in the original code. They shared the ExynosM3 ProcFamily. Adding it doesn't add a…
sdesmalenUnsubmitted
Done
ReplyInline Actions

Makes sense. Perhaps a comment would be useful in that case, e.g. /*Re-uses ExynosM3 proc family*/ or something like that.

sdesmalen: Makes sense. Perhaps a comment would be useful in that case, e.g. `/*Re-uses ExynosM3 proc…
[FeatureArithmeticBccFusion, [FeatureArithmeticBccFusion,
FeatureArithmeticCbzFusion, FeatureArithmeticCbzFusion,
FeatureExynosCheapAsMoveHandling, FeatureExynosCheapAsMoveHandling,
FeatureForce32BitJumpTables, FeatureForce32BitJumpTables,
FeatureFuseAddress, FeatureFuseAddress,
FeatureFuseAES, FeatureFuseAES,
FeatureFuseArithmeticLogic, FeatureFuseArithmeticLogic,
FeatureFuseCCSelect, FeatureFuseCCSelect,
▲ Show 20 Lines • Show All 406 Lines • Show Last 20 Lines