This is an archive of the discontinued LLVM Phabricator instance.

[ARM][AArch64] Add ARM specific builtin for clz that is not undefined for 0 in ubsan.
ClosedPublic

Authored by craig.topper on Jul 10 2023, 8:21 PM.

Details

Summary

D152023 made ubsan consider __builtin_clz of 0 undefined regardless of
the target. This ensures portability and matches gcc.

This causes the ACLE intrinsics to also be considered to also be
considered to be undefined for 0 since they used the generic builtins
as their implementation.

This patch adds builtins for ARM that ubsan doesn't know about to make
the behavior defined for 0. Alternatively, I could have added a zero
check to the intrinsics, but the dedicated builtin will give better -O0
codegen.

Fixes #63113.

Diff Detail

Event Timeline

craig.topper created this revision.Jul 10 2023, 8:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 10 2023, 8:21 PM
craig.topper requested review of this revision.Jul 10 2023, 8:21 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 10 2023, 8:21 PM
craig.topper added inline comments.Jul 10 2023, 8:24 PM
clang/test/CodeGen/arm_acle.c
351 ↗(On Diff #538901)

__builtin_clzl returned a signed int, but the new builtin returns an unsigned int so the cast changed.

I would prefer the simplicity of adding a check in the intrinsic itself, rather than adding the target-specific builtins. Slightly worse codegen at -O0 doesn't matter imho. However I don't feel very strongly about it, so if others are happy with this then LGTM.

Also thanks for fixing this!

tmatheson accepted this revision.Jul 12 2023, 6:05 AM
This revision is now accepted and ready to land.Jul 12 2023, 6:05 AM
This revision was landed with ongoing or failed builds.Jul 12 2023, 9:29 AM
This revision was automatically updated to reflect the committed changes.