This is an archive of the discontinued LLVM Phabricator instance.

[RISCV][ASAN] updated platform macros to simplify detection of RISCV64 platform
ClosedPublic

Authored by EccoTheDolphin on Sep 20 2020, 10:55 PM.

Details

Summary

[2/11] patch series to port ASAN for riscv64

Depends On D87997

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptSep 20 2020, 10:55 PM
Herald added subscribers: Restricted Project, evandro, luismarques and 12 others. · View Herald Transcript
EccoTheDolphin requested review of this revision.Sep 20 2020, 10:55 PM
EccoTheDolphin retitled this revision from [RISCV][ASAN] updated platform macros to simplify detection of RISCV64 to [RISCV][ASAN] updated platform macros to simplify detection of RISCV64 platform.Sep 20 2020, 11:44 PM
EccoTheDolphin edited the summary of this revision. (Show Details)
EccoTheDolphin edited the summary of this revision. (Show Details)

addressing comments, changing the order of modifications

EccoTheDolphin edited the summary of this revision. (Show Details)Sep 22 2020, 11:37 AM
eugenis added inline comments.Sep 22 2020, 1:20 PM
compiler-rt/lib/sanitizer_common/sanitizer_platform.h
222

Nit: Unnecessary braces around the expression.

compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
102

remove "defined"

addressing comments

EccoTheDolphin marked an inline comment as done.Sep 22 2020, 8:22 PM
EccoTheDolphin marked an inline comment as done.
vitalybuka accepted this revision.Sep 22 2020, 9:22 PM
vitalybuka added a subscriber: vitalybuka.
vitalybuka added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_platform.h
226

Please just apply clang-format if it conflicts with old code.

This revision is now accepted and ready to land.Sep 22 2020, 9:22 PM
EccoTheDolphin added inline comments.Sep 22 2020, 9:38 PM
compiler-rt/lib/sanitizer_common/sanitizer_platform.h
226

Please just apply clang-format if it conflicts with old code.

Just to double-check...

clang-format does indeed complain. I was under impression that it's better to keep the code style consistent with the existing codebase.

Is it not the case and the current policy is just to follow clang-format suggestions?

This revision was landed with ongoing or failed builds.Sep 22 2020, 9:44 PM
This revision was automatically updated to reflect the committed changes.
vitalybuka added inline comments.Sep 22 2020, 11:07 PM
compiler-rt/lib/sanitizer_common/sanitizer_platform.h
226

Please just apply clang-format if it conflicts with old code.

Just to double-check...

clang-format does indeed complain. I was under impression that it's better to keep the code style consistent with the existing codebase.

That's true if it does not conflicts with automation. It's not worth of this noise on code review and periodic hassle to undo of local clang-format results.
Just make sure to use corresponding .clang-format file and not a particular --style

Is it not the case and the current policy is just to follow clang-format suggestions?