diff mbox series

[v2] MIPS: Workaround clang inline compat branch issue

Message ID 20230228111917.82910-1-jiaxun.yang@flygoat.com (mailing list archive)
State Superseded
Headers show
Series [v2] MIPS: Workaround clang inline compat branch issue | expand

Commit Message

Jiaxun Yang Feb. 28, 2023, 11:19 a.m. UTC
Clang is unable to handle the situation that a chunk of inline
assembly ends with a compat branch instruction and then compiler
generates another control transfer instruction immediately after
this compat branch. The later instruction will end up in forbidden
slot and cause exception.

Workaround by add a option to control the use of compact branch.
Currently it's selected by CC_IS_CLANG and hopefully we can change
it to a version check in future if clang manages to fix it.

Fix boot on boston board.

Link: https://github.com/llvm/llvm-project/issues/61045
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
v2: Add Link tag to LLVM bug
---
 arch/mips/Kconfig           | 3 +++
 arch/mips/include/asm/asm.h | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

Comments

Nathan Chancellor Feb. 28, 2023, 7:21 p.m. UTC | #1
On Tue, Feb 28, 2023 at 11:19:17AM +0000, Jiaxun Yang wrote:
> Clang is unable to handle the situation that a chunk of inline
> assembly ends with a compat branch instruction and then compiler
> generates another control transfer instruction immediately after
> this compat branch. The later instruction will end up in forbidden
> slot and cause exception.
> 
> Workaround by add a option to control the use of compact branch.
> Currently it's selected by CC_IS_CLANG and hopefully we can change
> it to a version check in future if clang manages to fix it.
> 
> Fix boot on boston board.
> 
> Link: https://github.com/llvm/llvm-project/issues/61045
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>

Thanks for filing that bug and debugging this. I came across a post from
the (now former) MIPS code owner and I am not sure anyone else has
stepped up, the vast majority of commits to llvm/lib/Targets/Mips have
been either NFC commits or treewide refactorings, so it will be
interesting to see if we can get that fixed.

https://discourse.llvm.org/t/mips-backend-code-owner/60737
https://github.com/llvm/llvm-project/commit/7daed359111f6d151fef447f520f85ef1dabedf6

For now:

Acked-by: Nathan Chancellor <nathan@kernel.org>

If a v3 is necessary for some reason, a link to that issue in the source
itself would be useful, as we can easily grep for 'llvm/llvm-project' to
audit issues; I am not sure it is worth sending a v3 just for that alone
though.

> ---
> v2: Add Link tag to LLVM bug
> ---
>  arch/mips/Kconfig           | 3 +++
>  arch/mips/include/asm/asm.h | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 37072e15b263..adf2c5a0bdba 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -3206,6 +3206,9 @@ config CC_HAS_MNO_BRANCH_LIKELY
>  	def_bool y
>  	depends on $(cc-option,-mno-branch-likely)
>  
> +config CC_HAS_BROKEN_INLINE_COMPAT_BRANCH
> +	def_bool y if CC_IS_CLANG
> +
>  menu "Power management options"
>  
>  config ARCH_HIBERNATION_POSSIBLE
> diff --git a/arch/mips/include/asm/asm.h b/arch/mips/include/asm/asm.h
> index 336ac9b65235..2e99450f4228 100644
> --- a/arch/mips/include/asm/asm.h
> +++ b/arch/mips/include/asm/asm.h
> @@ -336,7 +336,7 @@ symbol		=	value
>   */
>  #ifdef CONFIG_WAR_R10000_LLSC
>  # define SC_BEQZ	beqzl
> -#elif MIPS_ISA_REV >= 6
> +#elif !defined(CONFIG_CC_HAS_BROKEN_INLINE_COMPAT_BRANCH) && MIPS_ISA_REV >= 6
>  # define SC_BEQZ	beqzc
>  #else
>  # define SC_BEQZ	beqz
> -- 
> 2.37.1 (Apple Git-137.1)
>
Jiaxun Yang Feb. 28, 2023, 7:28 p.m. UTC | #2
> 2023年2月28日 19:21,Nathan Chancellor <nathan@kernel.org> 写道:
> 
> On Tue, Feb 28, 2023 at 11:19:17AM +0000, Jiaxun Yang wrote:
>> Clang is unable to handle the situation that a chunk of inline
>> assembly ends with a compat branch instruction and then compiler
>> generates another control transfer instruction immediately after
>> this compat branch. The later instruction will end up in forbidden
>> slot and cause exception.
>> 
>> Workaround by add a option to control the use of compact branch.
>> Currently it's selected by CC_IS_CLANG and hopefully we can change
>> it to a version check in future if clang manages to fix it.
>> 
>> Fix boot on boston board.
>> 
>> Link: https://github.com/llvm/llvm-project/issues/61045
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> 
> Thanks for filing that bug and debugging this. I came across a post from
> the (now former) MIPS code owner and I am not sure anyone else has
> stepped up, the vast majority of commits to llvm/lib/Targets/Mips have
> been either NFC commits or treewide refactorings, so it will be
> interesting to see if we can get that fixed.

Yunqiang Su <syq@debian.org>[1] said he will take a look on that issue.

> 
> https://discourse.llvm.org/t/mips-backend-code-owner/60737
> https://github.com/llvm/llvm-project/commit/7daed359111f6d151fef447f520f85ef1dabedf6
> 
> For now:
> 
> Acked-by: Nathan Chancellor <nathan@kernel.org>
> 
> If a v3 is necessary for some reason, a link to that issue in the source
> itself would be useful, as we can easily grep for 'llvm/llvm-project' to
> audit issues; I am not sure it is worth sending a v3 just for that alone
> though.

Will do a v3, it also helps me document the actual issue.

Thanks

[1]: https://reviews.llvm.org/p/wzssyqa/

- Jiaxun

>
diff mbox series

Patch

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 37072e15b263..adf2c5a0bdba 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -3206,6 +3206,9 @@  config CC_HAS_MNO_BRANCH_LIKELY
 	def_bool y
 	depends on $(cc-option,-mno-branch-likely)
 
+config CC_HAS_BROKEN_INLINE_COMPAT_BRANCH
+	def_bool y if CC_IS_CLANG
+
 menu "Power management options"
 
 config ARCH_HIBERNATION_POSSIBLE
diff --git a/arch/mips/include/asm/asm.h b/arch/mips/include/asm/asm.h
index 336ac9b65235..2e99450f4228 100644
--- a/arch/mips/include/asm/asm.h
+++ b/arch/mips/include/asm/asm.h
@@ -336,7 +336,7 @@  symbol		=	value
  */
 #ifdef CONFIG_WAR_R10000_LLSC
 # define SC_BEQZ	beqzl
-#elif MIPS_ISA_REV >= 6
+#elif !defined(CONFIG_CC_HAS_BROKEN_INLINE_COMPAT_BRANCH) && MIPS_ISA_REV >= 6
 # define SC_BEQZ	beqzc
 #else
 # define SC_BEQZ	beqz