[llvmlinux] [PATCH] x86/hweight: LLVMLinux: Fix __arch_hweight{32, 64}() for compilation with clang

[Sanjoy Das]

I'm an outsider to the llvmlinux project, so I cannot comment on the 
patch as is; as I do not know how what is a good candidate for merging 
into what tree and what isn't.  Sedat CC'ed me only because we had a 
discussion about this issue on IRC.

However, I will comment that clang not supporting ALTERNATIVE is only 
one issue here.  I think there is a bug that needs to be fixed in 
__arch_hweight64 --

	asm (ALTERNATIVE("call __sw_hweight64", POPCNT64, X86_FEATURE_POPCNT)
		     : "="REG_OUT (res)
		     : REG_IN (w));


does not specify that the "call __sw_hweight64" instruction may clobber 
caller saved registers (like %rcx and %rdx).

As far as I can tell, the function "works" with GCC only by accident. 
When compiling with either clang or GCC, __arch_hweight64 gets inlined 
into __bitmap_weight.  clang happens to regalloc "const unsigned long 
*bitmap" to %rcx while GCC happens to regalloc the same thing to %rdx; 
and it just so happens that __sw_hweight64 clobbers %rcx and not %rdx. 
Depending on the phase of the moon, __sw_hweight64 could easily be 
clobbering %rdx instead of %rcx (both are caller saved register, and can 
be potentially be clobbered across function calls), and in that case the 
kernel panic Sedat is trying to fix will show up on a GCC-built kernel 
as well.  Does this theory sound plausible?

Thanks,
-- Sanjoy


Sedat Dilek wrote:
> On Sun, Sep 13, 2015 at 6:50 PM, Jan-Simon Moeller<dl9pf at gmx.de>  wrote:
>> Hi,
>>
>>
>> Am Sonntag, 13. September 2015, 17:20:24 schrieb Sedat Dilek:
>>> Adapted partly from the original patch (see [1]).
>>> This patch was formerly together with [2] and provided as a single
>> patch.
>>> To quote [3]:
>>> "interesting, Jan-Simon said that patch was still part of the llvmlinux
>>> tree (it is really mandatory for clang) so i'm not sure what's going
>> on."
>>
>> Read the code, Luke.
>> We still patch it, but in the Makefile: see [1]
>>
>
> That seems not to be enough - except you compile with OptLevel -O0 or -O1.
>
>> [1]
>> http://git.linuxfoundation.org/?p=llvmlinux.git;a=blob;f=arch/x86_64/patches/hweight-x86.patch;h=959ec0711e84b195632f63a2f0ddd0c31f3c14d7;hb=HEAD
>>
>>
>>> See also [3] for an explanation of PaX Team on how to do a proper fix.
>> What might manifest is an issue with the asm ALTERNATIVE.
>> This is _already_ filed as
>>
>> https://llvm.org/bugs/show_bug.cgi?id=24487
>>
>
> I did not build with integrated-as compiler-flag, so can you explain
> what this BR is dealing with this issue?
>
>
>> Please comment there and help to get it fixed.
>>
>
>> Nack on the patch as it won't go upstream like this in the linux kernel.
>>
>
> Again, this patch is needed for a successful x86_64, you did not lost
> one word on it.
> AFAICS -no-integrated-as is default.
> With GCC v4.9 this workaround works well, just to let you know.
>
> Finally, will you reactivate that patch in arch/x86_64?
>
> - Sedat -