This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][MC] Refactor MC Code Emitter to avoid using magic values
ClosedPublic

Authored by dp on Nov 23 2022, 11:00 AM.

Diff Detail

Event Timeline

dp created this revision.Nov 23 2022, 11:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 23 2022, 11:00 AM
dp requested review of this revision.Nov 23 2022, 11:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 23 2022, 11:00 AM

I believe the latest guidance is to use std::Optional instead of llvm::Optional (https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716). Also it makes a difference in tests, so is not NFC. Otherwise looks good.

dp updated this revision to Diff 477578.Nov 23 2022, 12:16 PM
dp retitled this revision from [AMDGPU][MC][NFC] Refactor MC Code Emitter to avoid using magic values to [AMDGPU][MC] Refactor MC Code Emitter to avoid using magic values.

Corrected as suggested by Joe.
Joe, thanks for your valuable comments!

Joe_Nash accepted this revision.Nov 23 2022, 12:23 PM

Thanks to Jay as well for pointing out std::optional on another review.
LGTM

This revision is now accepted and ready to land.Nov 23 2022, 12:23 PM
foad added inline comments.Nov 23 2022, 12:49 PM
llvm/lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp
237

You can return {} if you prefer.

dp marked an inline comment as done.Nov 24 2022, 1:44 AM
dp added inline comments.
llvm/lib/Target/AMDGPU/MCTargetDesc/SIMCCodeEmitter.cpp
237

Thanks, I like it much better than nullopt.

dp updated this revision to Diff 477704.Nov 24 2022, 1:45 AM
dp marked an inline comment as done.

Correct as suggested by Jay.

This revision was landed with ongoing or failed builds.Nov 25 2022, 7:02 AM
This revision was automatically updated to reflect the committed changes.