[RISCV][crt] support building without init_array

[EccoTheDolphin]

rebasing patchset...

[kito-cheng]

RISC-V LLVM and GCC are default using init_array, curious why we need this?

[EccoTheDolphin]

In D87997#2289211, @kito-cheng wrote:

RISC-V LLVM and GCC are default using init_array, curious why we need this?

Actually, yes. You are correct - this one is not needed for ASAN support... My understanding is that this may be needed if we link against some custom runtime. I'll adjust the description of the patch.

[vitalybuka]

Actually I not appropriate reviewer for this one

[kito-cheng]

In D87997#2291675, @EccoTheDolphin wrote:

In D87997#2289211, @kito-cheng wrote:

RISC-V LLVM and GCC are default using init_array, curious why we need this?

Actually, yes. You are correct - this one is not needed for ASAN support... My understanding is that this may be needed if we link against some custom runtime. I'll adjust the description of the patch.

I would prefer let those custom runtime to using their crt files, otherwise that add extra 18 byte(auipc+jal+auipc+jal+c.nop) or 20 byte (auipc+jal+auipc+jal+nop) for all RISC-V program which using compiler-rt.

[EccoTheDolphin]

In D87997#2292015, @kito-cheng wrote:

In D87997#2291675, @EccoTheDolphin wrote:

In D87997#2289211, @kito-cheng wrote:

RISC-V LLVM and GCC are default using init_array, curious why we need this?

Actually, yes. You are correct - this one is not needed for ASAN support... My understanding is that this may be needed if we link against some custom runtime. I'll adjust the description of the patch.

I would prefer let those custom runtime to using their crt files, otherwise that add extra 18 byte(auipc+jal+auipc+jal+c.nop) or 20 byte (auipc+jal+auipc+jal+nop) for all RISC-V program which using compiler-rt.

But this logic is enabled only if CRT_HAS_INITFINI_ARRAY is not defined. Which is controlled by cmake (configuration script). In addition, other architectures (x86, sparc, arm, aarch64, sparc, powerpc) - do allow such fallback option. It just seems strange that RISCV does not have one.

[kito-cheng]

But this logic is enabled only if CRT_HAS_INITFINI_ARRAY is not defined. Which is controlled by cmake (configuration script). In addition, other architectures (x86, sparc, arm, aarch64, sparc, powerpc) - do allow such fallback option. It just seems strange that RISCV does not have one.

Oh, I didn't noticed that #elif is associated with #ifdef CRT_HAS_INITFINI_ARRAY, thanks your clarify, that's LGTM now.

[phosek]

LGTM

compiler-rt/lib/crt/crtbegin.c

120

Do you need the nop here? It's not used for __do_init call.

[luismarques]

LGTM. Just remove nop, ensure it isn't actually needed (rerun the tests, etc.), and feel free to commit this patch. Thanks!

compiler-rt/lib/crt/crtbegin.c

120

I agree the nop doesn't seem to be needed, and so should be removed. I'm not sure why it was added to the powerpc implementation, but I suspect that it's for branch relaxation.

[EccoTheDolphin]

@phosek, @luismarques thanks for your feedback (sorry for the delay with reply)!

Yes, the nop is not really needed. I'll make appropriate adjustments.

[luismarques]

In D87997#2320775, @EccoTheDolphin wrote:

@phosek, @luismarques thanks for your feedback (sorry for the delay with reply)!

Yes, the nop is not really needed. I'll make appropriate adjustments.

@EccoTheDolphin ping?

[EccoTheDolphin]

removing unnecessary nop

[MaskRay]

This does not look right to me. Newer architectures like aarch64/risc-v do not need to have .ctors/.dtors support. Can you investigate why -DCRT_HAS_INITFINI_ARRAY isn't set in your build instead?

[luismarques]

In D87997#2426709, @MaskRay wrote:

This does not look right to me. Newer architectures like aarch64/risc-v do not need to have .ctors/.dtors support. Can you investigate why -DCRT_HAS_INITFINI_ARRAY isn't set in your build instead?

ping @EccoTheDolphin?

compiler-rt/lib/crt/crtbegin.c

 
 #ifdef CRT_HAS_INITFINI_ARRAY
 __attribute__((section(".init_array"),
                used)) static void (*__init)(void) = __do_init;
 #elif defined(__i386__) || defined(__x86_64__)
 __asm__(".pushsection .init,\"ax\",@progbits\n\t"
     "call " __USER_LABEL_PREFIX__ "__do_init\n\t"
     ".popsection");
+#elif defined(__riscv)
+__asm__(".pushsection .init,\"ax\",%progbits\n\t"
+        "call " __USER_LABEL_PREFIX__ "__do_init\n\t"
+        ".popsection");
 #elif defined(__arm__) || defined(__aarch64__)
 __asm__(".pushsection .init,\"ax\",%progbits\n\t"
     "bl " __USER_LABEL_PREFIX__ "__do_init\n\t"
     ".popsection");
 #elif defined(__powerpc__) || defined(__powerpc64__)
 __asm__(".pushsection .init,\"ax\",@progbits\n\t"
     "bl " __USER_LABEL_PREFIX__ "__do_init\n\t"
     "nop\n\t"

 __asm__(".pushsection .fini,\"ax\",%progbits\n\t"
     "bl " __USER_LABEL_PREFIX__ "__do_fini\n\t"
     ".popsection");
 #elif defined(__powerpc__) || defined(__powerpc64__)
 __asm__(".pushsection .fini,\"ax\",@progbits\n\t"
     "bl " __USER_LABEL_PREFIX__ "__do_fini\n\t"
     "nop\n\t"
     ".popsection");
+#elif defined(__riscv)
+__asm__(".pushsection .fini,\"ax\",@progbits\n\t"
+        "call " __USER_LABEL_PREFIX__ "__do_fini\n\t"
+        ".popsection");
 #elif defined(__sparc__)
 __asm__(".pushsection .fini,\"ax\",@progbits\n\t"
     "call " __USER_LABEL_PREFIX__ "__do_fini\n\t"
     ".popsection");
 #else
 #error "crtbegin without .init_fini array unimplemented for this architecture"
 #endif  // CRT_HAS_INIT_FINI_ARRAY