Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LLVM backend: on Windows use clang instead of llc #279

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rversteegen
Copy link
Member

@rversteegen rversteegen commented Jan 5, 2021

llc.exe is not included with the LLVM official Windows binaries. clang can
compile .ll files too but requires different args.

This behaviour is gated by __FB_WIN32__ because there doesn't seem to be an
existing way to check whether a tool exists. It would be better to fallback to
clang only if llc doesn't exist (for example I see it is missing from at least
some Android NDK toolchains too).

Tested on Linux, not Windows.

C.f. https://www.freebasic.net/forum/viewtopic.php?p=278824#p278824

@zamabuvaraeu
Copy link
Contributor

I believe FreeBASIC should use LLMV directly, not indirectly through CLang.

@jayrm
Copy link
Member

jayrm commented Jan 9, 2021

With a few fixups, I was able to compile and run a simple program on windows using llc.exe. I don't know what changes would be needed for clang.exe on windows - it didn't work for me.

Following is the results of what didn't work, and then what did work.

In this PR:
#if __FB_WIN32__ should be #ifdef __FB_WIN32__

I then tried a simple program

print "hello"

I tried clang, sort of ...

Since I already had emscripten SDK installed and built I just grabbed 'clang.exe' version 12.0.0 from there. Maybe it's not quite the right build.

$ fbc-win32.exe hello.bas -gen llvm -v

With the following results:

FreeBASIC Compiler - Version 1.08.0 (2021-01-09), built for win32 (32bit)
Copyright (C) 2004-2019 The FreeBASIC development team.
standalone
target:       win32, 486, 32bit
backend:      llvm
compiling:    hello.bas -o hello.ll (main module)
compiling LLVM IR:          d:\fb.git\bin\win32\clang.exe -S -Wno-override-module -no-integrated-as --target=i686 -O0 -masm=intel "hello.ll" -o "hello.asm"
assembling:   d:\fb.git\bin\win32\as.exe --32 --strip-local-absolute "hello.asm" -o "hello.o"
hello.asm: Assembler messages:
hello.asm:6: Warning: .type pseudo-op used outside of .def/.endef: ignored.
hello.asm:6: Error: junk at end of line, first unrecognized character is `_'
hello.asm:51: Warning: .size pseudo-op used outside of .def/.endef: ignored.
hello.asm:51: Error: junk at end of line, first unrecognized character is `_'
hello.asm:53: Warning: .type pseudo-op used outside of .def/.endef: ignored.
hello.asm:53: Error: junk at end of line, first unrecognized character is `_'
... etc
... etc

The system as (binutils) on windows doesn't seem to be able to handle all the directives in the asm.

Tried llc.exe and almost worked.

Next I tried disabling the NO_LLC directive in this PR and just used llc.exe version 12.0.0 also from the emscripten SDK.

$ fbc-win32.exe hello.bas -gen llvm -v
With the following results:

FreeBASIC Compiler - Version 1.08.0 (2021-01-09), built for win32 (32bit)
Copyright (C) 2004-2019 The FreeBASIC development team.
standalone
target:       win32, 486, 32bit
backend:      llvm
compiling:    hello.bas -o hello.ll (main module)
compiling LLVM IR:          d:\fb.git\bin\win32\llc.exe -march=x86 -O0 --x86-asm-syntax=intel "hello.ll" -o "hello.asm"
assembling:   d:\fb.git\bin\win32\as.exe --32 --strip-local-absolute "hello.asm" -o "hello.o"
linking:      d:\fb.git\bin\win32\ld.exe -m i386pe -o "hello.exe" -subsystem console "d:\fb.git\lib\win32\fbextra.x" --stack 1048576,1048576 -s -L "d:\fb.git\li
b\win32" -L "." "d:\fb.git\lib\win32\crt2.o" "d:\fb.git\lib\win32\crtbegin.o" "d:\fb.git\lib\win32\fbrt0.o" "hello.o" "-(" -lfb -lgcc -lmsvcrt -lkernel32 -luser
32 -lmingw32 -lmingwex -lmoldname -lgcc_eh "-)" "d:\fb.git\lib\win32\crtend.o"
hello.o:fake:(.text+0x41): undefined reference to `_fb_Init@12@12'
hello.o:fake:(.text+0x5a): undefined reference to `_fb_StrAllocTempDescZEx@8@8'
hello.o:fake:(.text+0x77): undefined reference to `_fb_PrintString@12@12'
hello.o:fake:(.text+0x88): undefined reference to `_fb_End@4@4'
d:\fb.git\lib\win32/libmingw32.a(lib32_libmingw32_a-crt0_c.o):crt0_c.c:(.text.startup+0x39): undefined reference to `WinMain@16'
linking failed: 'd:\fb.git\bin\win32\ld.exe' terminated with exit code 1

Fixed (probably?) the name mangling to work with with llc.exe

Because fbc is already doing the name mangling, need to escape the name with "\001" so that llc doesn't mangle the already mangled name.

Once I made this change I was able to compile and run the simple print "hello" program.

@rversteegen
Copy link
Member Author

Thanks for the testing. I tried cross-compiling to win32 from linux and was able to reproduce those errors. I realised from a web search that the problem is clang outputting directives which are not supported by mingw, because it defaults to a platform other than win32 (including presumably clang from emscripten installed on Windows). Passing a more complete --target to clang fixes the problem (and it then fails due to the name mangling). I'm going to also rework this patch to do a runtime fallback to clang instead of hardcoding on win32.

As for the #if, oops.

I don't understand how you escaped the names to fix the name mangling, and it's totally independent of this patch. Could you please commit the fix for that separately?

@rversteegen
Copy link
Member Author

rversteegen commented Jan 10, 2021

Actually I guess your escaping of the names was a hack, and we instead just need to find the right place to disable fbc adding the suffix?

@jayrm
Copy link
Member

jayrm commented Jan 10, 2021

I'm working on a fix for escaping the procedure names in symb-mangling.bas:hMangleProc().

@jayrm
Copy link
Member

jayrm commented Jan 10, 2021

I think we need to escape global names for windows targets on LLVM.

  • For asm we always use the mangled name as-is.
  • For gcc backend we sort of let gcc do it but also supply an asm alias where the gcc might get it wrong calculating the suffix
  • I didn't see a way to do an alias in llvm.
  • escaping also for names without a @N suffix, I think due to the handling of the leading underscore.

Using this method from https://llvm.org/docs/LangRef.html for escaping.

Symbols prefixed with the mangling escape character \01 are passed through directly to the assembler without the escape character

@jayrm
Copy link
Member

jayrm commented Jan 10, 2021

@rversteegen

I'm going to also rework this patch to do a runtime fallback to clang instead of hardcoding on win32.

FYI, I was thinking of something kind of like the following. I have this feeling that will need to track more information about tools and how to run them and a table might help that can be extended. Additional fields could be added instead having only the last tool info cached in fbcFindBin().

enum FBCTOOL
	FBCTOOL_AS = 0
	FBCTOOL_AR
	FBCTOOL_LD
	FBCTOOL_GCC
	FBCTOOL_LLC
	FBCTOOL_CLANG
	FBCTOOL_DLLTOOL
	FBCTOOL_GORC
	FBCTOOL_WINDRES
	FBCTOOL_CXBE
	FBCTOOL_DXEGEN
	FBCTOOL_EMAS
	FBCTOOL_EMAR
	FBCTOOL_EMLD
	FBCTOOL_EMCC
	FBCTOOL__COUNT
end enum

enum FBCTOOLFLAG
	FBCTOOLFLAG_MISSING = 0
	FBCTOOLFLAG_EXISTS = 1
	FBCTOOLFLAG_CHECK = 2
end enum

type FBCTOOLINFO
	name as zstring * 16
	flags as FBCTOOLFLAG
end type

'' must be same order as enum FBCTOOL
static shared as FBCTOOLINFO fbctoolTB(0 to FBCTOOL__COUNT-1) = _
{ _
	/' FBCTOOL_AS      '/ ( "as"     , FBCTOOLFLAG_EXISTS  ), _
	/' FBCTOOL_AR      '/ ( "ar"     , FBCTOOLFLAG_EXISTS  ), _
	/' FBCTOOL_LD      '/ ( "ld"     , FBCTOOLFLAG_EXISTS  ), _
	/' FBCTOOL_GCC     '/ ( "gcc"    , FBCTOOLFLAG_EXISTS  ), _
	/' FBCTOOL_LLC     '/ ( "llc"    , FBCTOOLFLAG_EXISTS  ), _
	/' FBCTOOL_CLANG   '/ ( "clang"  , FBCTOOLFLAG_EXISTS  ), _
	/' FBCTOOL_DLLTOOL '/ ( "dlltool", FBCTOOLFLAG_EXISTS  ), _
	/' FBCTOOL_GORC    '/ ( "GoRC"   , FBCTOOLFLAG_EXISTS  ), _
	/' FBCTOOL_WINDRES '/ ( "windres", FBCTOOLFLAG_EXISTS  ), _
	/' FBCTOOL_CXBE    '/ ( "cxbe"   , FBCTOOLFLAG_EXISTS  ), _
	/' FBCTOOL_DXEGEN  '/ ( "dxe3gen", FBCTOOLFLAG_EXISTS  ), _
	/' FBCTOOL_EMAS    '/ ( "emcc"   , FBCTOOLFLAG_EXISTS  ), _
	/' FBCTOOL_EMAR    '/ ( "emar"   , FBCTOOLFLAG_EXISTS  ), _
	/' FBCTOOL_EMLD    '/ ( "emcc"   , FBCTOOLFLAG_EXISTS  ), _
	/' FBCTOOL_EMCC    '/ ( "emcc"   , FBCTOOLFLAG_EXISTS  )  _
}

@rversteegen
Copy link
Member Author

FYI, I was thinking of something kind of like the following.

So are you working on this?
Would an initial value of FBCTOOLFLAG_EXISTS mean that it should be assumed to exist, so a compile error is thrown if it turns out it doesn't?

Another usecase is that (on FreeBSD) it may be necessary to ask gcc what the correct path to as and ld are, rather than just running as/ld. I was just going to put that as a special case in fbcFindBin.

@jayrm
Copy link
Member

jayrm commented Jan 10, 2021

I started to work on it when reviewing this PR to remove the #ifdef __FB_WIN32__ by setting a flag at runtime. What I posted above is about as far as I got, with a few changes in fbcFindBin(). If you like, I could add in the framework for the change, and then you could extend it with your own flags, etc. Or if you prefer, copy / paste it in and go to town.

Yes, initial value of FBCTOOLFLAG_EXISTS just assumes that the tool exists more or less like what we have now. Then change the initial flags values, add additional flags, or set flags at fbc host run-time to control other tool behaviours, cache tool names & paths as I should expect that those should not change over the course of a single fbc invocation once everything has been initialised or queried.

For specific behaviours like for FreeBSD: if it's host specific, then I suppose that could be #ifdef's. For anything else it probably needs to be a special case run-time check of env.clopt.target or env.target.options, etc. Either directly where needed, or indirectly by setting a flag in fbctoolTB() and checking the flag later.

@jayrm
Copy link
Member

jayrm commented Jan 10, 2021

@rversteegen another option if you interested in wrapping up this PR. Is just make the changes for clang, and can deal with fbctoolTB() changes afterwards.

I've pushed changes to fbc/master for escaping procedure names on llvm+win32 [12a4d28]

@jayrm
Copy link
Member

jayrm commented Aug 7, 2021

I've made changes in #334 to modify fbc's internals for tool usage.
I'm going to force push a rebase for this PR and merge it in.

@jayrm
Copy link
Member

jayrm commented Aug 7, 2021

Oh wait, hold on. I'm still going to do the rebase. This needs more testing for windows.
llc.exe from emscripten produces a valid executable when using '-gen llvm'
using clang.exe so for me produces compile errors - bad directives to the assembler.
More investigation and testing is needed.

llc.exe is not included with the LLVM official Windows binaries. clang can
compile .ll files too but requires different args.

This behaviour is gated by __FB_WIN32__ because there doesn't seem to be an
existing way to check whether a tool exists. It would be better to fallback to
clang only if llc doesn't exist (for example I see it is missing from at least
some Android NDK toolchains too).

Tested on Linux, not Windows.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants