1477929 - [LTO] [arm] <inline asm>:2:9: error: operand must be a register in range [r0, r7]

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Mike Hommey [:glandium]]

libfreebl3.so fails to build on arm with LTO enabled with the following error:

[task 2018-07-19T08:57:57.985Z] 08:57:57     INFO -  <inline asm>:2:9: error: operand must be a register in range [r0, r7]
[task 2018-07-19T08:57:57.985Z] 08:57:57     INFO -  cbz     r10, 2f
[task 2018-07-19T08:57:57.985Z] 08:57:57     INFO -          ^
[task 2018-07-19T08:57:57.985Z] 08:57:57     INFO -  LLVM ERROR: Error parsing inline asm

It's not clear from the ARM documentation for CBZ whether that's a problem with llvm or a limitation of the CBZ instruction, but at least the version of LLVM we use insists that CBZ is used with a core register (between r0 and r7, included).

That instruction comes from inline asm blocks, and in normal builds, it just happens to work because the variable that is passed to CBZ is a function argument, one of the four first arguments, such that it is register allocated. The compiler just decides to use that register directly.

With LTO, the function actually ends up inlined in its caller, and the register allocated to the variable can end up outside the core register range.

The GCC documentation for machine-constraints indicates the `l` constraint exists to limit the possible registers:
"In Thumb State the core registers r0-r7. In ARM state this is an alias for the r constraint."

(CBZ being only used on thumb)

Comment 1

[Mike Hommey [:glandium]]

Attachment: Use a stricter constraint for the register passed to cbz

I tried putting this in phabricator, but it's stuck on "Wait Until Bugzilla Updates Revision."

Comment 2

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8994439 [details] [diff] [review]
Use a stricter constraint for the register passed to cbz

Review of attachment 8994439 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm but shouldn't the same issue occur then in s_mpv_mul_d_add_prop?
We don't have arm machines on CI (only aarch64) so I can't test the fix. Do you have a link to the failing build?

Comment 3

[Mike Hommey [:glandium]]

s_mpv_mul_d_add_prop is harcoding the register. So is s_mpv_sqr_add_prop.

Comment 4

[Mike Hommey [:glandium]]

failing build: https://treeherder.mozilla.org/logviewer.html#?job_id=188897260&repo=try
the patch allows to go further (and fail on linking libxul): https://treeherder.mozilla.org/logviewer.html#?job_id=189713762&repo=try

Comment 5

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8994439 [details] [diff] [review]
Use a stricter constraint for the register passed to cbz

Review of attachment 8994439 [details] [diff] [review]:
-----------------------------------------------------------------

> s_mpv_mul_d_add_prop is harcoding the register. So is s_mpv_sqr_add_prop.

Ah right.

Thanks!

Comment 6

[Mike Hommey [:glandium]]

Attachment: Bug 1477929 - Use a stricter constraint for the register passed to cbz.

libfreebl3.so fails to build on arm with LTO enabled with the following
error:

     INFO -  <inline asm>:2:9: error: operand must be a register in range [r0, r7]
     INFO -  cbz     r10, 2f
     INFO -          ^
     INFO -  LLVM ERROR: Error parsing inline asm

It's not clear from the ARM documentation for CBZ whether that's a
problem with llvm or a limitation of the CBZ instruction, but at least
the version of LLVM we use insists that CBZ is used with a core register
(between r0 and r7, included).

That instruction comes from inline asm blocks, and in normal builds, it
just happens to work because the variable that is passed to CBZ is a
function argument, one of the four first arguments, such that it is
register allocated. The compiler just decides to use that register
directly.

With LTO, the function actually ends up inlined in its caller, and the
register allocated to the variable can end up outside the core register
range.

The GCC documentation for machine-constraints indicates the `l`
constraint exists to limit the possible registers:
 "In Thumb State the core registers r0-r7. In ARM state this is an alias
for the r constraint."

(CBZ being only used on thumb)

Comment 7

[Mike Hommey [:glandium]]

https://hg.mozilla.org/projects/nss/rev/64d443c8fcc7897ee7a3317d258330649c59dbdb