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

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

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

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

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/
4 lines

Diff 411820

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", def TuneExynosM4 : SubtargetFeature<"exynosm4", "ARMProcFamily", "ExynosM3",
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…
"Samsung Exynos-M3 processors", "Samsung Exynos-M4 processors",
[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