Skip to content

Conversation

@czsgaba
Copy link
Contributor

@czsgaba czsgaba commented May 9, 2019

aarch64 is not a permissible value for -march

From the GCC manual:
-march=name
Specify the name of the target architecture and, optionally, one or more feature
modifiers. This option has the form ‘-march=arch{+[no]feature}*’.
The permissible values for arch are ‘armv8-a’, ‘armv8.1-a’, ‘armv8.2-a’,
‘armv8.3-a’, ‘armv8.4-a’, ‘armv8.5-a’ or native.
The value ‘armv8.5-a’ implies ‘armv8.4-a’ and enables compiler support for
the ARMv8.5-A architecture extensions.
The value ‘armv8.4-a’ implies ‘armv8.3-a’ and enables compiler support for
the ARMv8.4-A architecture extensions.
The value ‘armv8.3-a’ implies ‘armv8.2-a’ and enables compiler support for
the ARMv8.3-A architecture extensions.
The value ‘armv8.2-a’ implies ‘armv8.1-a’ and enables compiler support for
the ARMv8.2-A architecture extensions.
The value ‘armv8.1-a’ implies ‘armv8-a’ and enables compiler support for the
ARMv8.1-A architecture extension. In particular, it enables the ‘+crc’, ‘+lse’,
and ‘+rdma’ features.
The value ‘native’ is available on native AArch64 GNU/Linux and causes the
compiler to pick the architecture of the host system. This option has no effect
if the compiler is unable to recognize the architecture of the host system,
The permissible values for feature are listed in the sub-section on [‘-march’ and
‘-mcpu’ Feature Modifiers], page 250. Where conflicting feature modifiers are
specified, the right-most feature is used.
GCC uses name to determine what kind of instructions it can emit when generating assembly code. If ‘-march’ is specified without either of ‘-mtune’ or
‘-mcpu’ also being specified, the code is tuned to perform well across a range of
target processors implementing the target architecture.

@rversteegen
Copy link
Member

arm8-a is the first ARM version to introduce the AArch64 instruction set, so that's correct.

BUT arm8-a does not imply 64 bit, while aarch64 does. Both gcc and llvm backends apparently rely on the -march=aarch64 flag to tell the compiler to produce 64 bit code. -m64 is passed to gcc only when targetting x86_64, and the llvm backend never receives -m64. So this needs to be fixed, or else this patch appears to be wrong. I haven't actually tested it though.

Also, this patch changes only -march flag passed to llvm, not gcc. The latter is in cputypeinfo() in fb.bas.

@rversteegen
Copy link
Member

Well I was a little hasty in saying this patch is wrong, because passing -march=aarch64 is even more wrong: it doesn't work. I just checked. I thought I've compiled FB code for aarch4 in the past, but apparently I was mistaken.

@czsgaba
Copy link
Contributor Author

czsgaba commented May 10, 2019

OK I was too hurry with the patch.
Because you're right ,,this patch changes only -march flag passed to llvm, not gcc."

-march flag passed to GCC must be repaired too.
-march=armv8-a maybe is not the best value for aarch64.

But something must be done with it becouse -march=aarch64 is wrong and doesn't work.

@czsgaba czsgaba changed the title Update fbc.bas Fix compilation error on/for aarch64 (arm64) May 14, 2019
@rversteegen
Copy link
Member

New commit looks good.
__fb_con.has_perm tells whether the port input/output asm instructions (inb, outb) can be used without generating an exception (apparently a segmentation fault according to the outb man page). Note that vga16_blitter in gfx_driver_fbdev.c uses outb (rather interesting...).

@dkl
Copy link
Member

dkl commented May 15, 2019

Agreed, the gcc option in fb.bas (the entry in the cputypeinfo table) needs to be updated too.

Using armv8-a to make it work is good enough at first, but chances are fbc needs to learn some additional AArch64 types besides this one...

@czsgaba
Copy link
Contributor Author

czsgaba commented May 15, 2019

Ok i am happy that second commit looks good.
I don't want to change -march flag passed to llvm now. I want to fix GCC.
Is there a possibility to delete the first commit (428fc9a)? (I'm new on Github)

I did some tests with default settings of GCC on different platforms.

gcc -v test.c

on armfh:
COLLECT_GCC_OPTIONS='-v' '-march=armv7-a' '-mfloat-abi=hard' '-mfpu=vfpv3-d16' '-mthumb' '-mtls-dialect=gnu'
on arm64:
COLLECT_GCC_OPTIONS='-v' '-mlittle-endian' '-mabi=lp64'
on amd64:
COLLECT_GCC_OPTIONS='-v' '-mtune=generic' '-march=x86-64'

so -march= flag is not set by default on arm64.
(Raspberry Pi 3, ARMv8 64-bit system ubuntu-mate https://ubuntu-mate.org/download/)

then I tried:

gcc hw.c -v -march=native

COLLECT_GCC_OPTIONS='-v'  '-mlittle-endian' '-mabi=lp64' '-march=armv8-a+crc'

-mabi=lp64
-mabi=name
Generate code for the specified data model. Permissible values are ‘ilp32’ for SysV-like data model where int, long int and pointers are 32 bits, and ‘lp64’ for SysV-like data model where int is 32 bits, but long int and pointers are 64 bits.

The default depends on the specific target configuration. Note that the LP64 and ILP32 ABIs are not link-compatible; you must compile your entire program with the same ABI, and link with a compatible set of libraries.

https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/AArch64-Options.html#AArch64-Options

@rversteegen
Copy link
Member

You should pass -m64 to gcc/clang for 64-bit builds, which will change ABIs, available instructions, libraries, and whatever else is required, rather than to use a narrow low-level flag like -mabi. (-m64 will only be required if the compiler is built for both 32- and 64-bit builds.)

To replace existing commits in a github pull request, do a forced push to the branch on github that you're requesting a pull from. E.g. git push -f <remotename>
There are two alternatives to that: leave the existing commits but add more to fix up problems with them, or open a whole new pull request. Some projects/maintainers are opinionated about that sort of thing.

@jayrm
Copy link
Member

jayrm commented Jun 1, 2019

@czsgaba, is this ready to merge? As @rversteegen mentioned, if you want to replace your commits for this pull request, you must make the changes on your local branch, and then force push your branch; it will overwrite the commits. I'm not in any position to test this; so I will simply trust you that it is ready to merge in. As @dkl pointed out, this change is OK for now, but needs more work down the road.

@czsgaba
Copy link
Contributor Author

czsgaba commented Jul 9, 2019

Sorry but had too much work to rebuild (reconstruct) my house.
At the same time came out Raspberry Pi 4 with 1,2,4 GB of RAM! But there is not yet Linux distro with support for aarch64 on it. Ubuntu MATE announced that they are preparing aarch64 version for Raspberry Pi 4. So I want to wait and test it.

So I think thet this is ready to merge. Becouse it solves the problem with compiling fbc on aarch64 (#include <sys/io.h>).
And in the future problem with -march flag passed to GCC must be solved completely.

czsgaba added 2 commits July 14, 2019 09:15
…options to gcc/LLVM

- aarch64 is not a permissible value for -march passed to gcc/LLVM
- fbc will accept aarch64, then map to '-march=armv8-a' when calling the backend
- comments in fbc.bas:hCompileStage2Module() listing permissible values
- fix rtlib compilation error on/for aarch64 (arm64) or other platforms
- use of sys/io.h is only to call ioperm(), and determine permission for direct use out in/out instructions
- sys/io.h is used only for x86, x86_64 and is unnecessary for all other linux targets (armhf ,arm64, mips ....)
@jayrm
Copy link
Member

jayrm commented Jul 14, 2019

@czsgaba, I have cleaned up this pull request.

  • rebased on to current master (as mentioned by @rversteegen)
  • added changelog.txt messages
  • cleaned up the commit messages, adding some of the information to source code comments. Not really the place for documentation or questions, but is still good information.
  • added "armv8-a" to the cputypeinfo() table. So, fbc will still take "aarch64" as the target, but map it to "armv8-a" for both gcc & LLVM (as mentioned by @dkl)

My apologies for the force push on your master. I was really curious to see if github would actually allow it, and it did. If you have continued work on your local repo master branch, take care to create another branch name for it before force pulling from your github repo. My opinion is to always create an alternate branch for development and publish that one for pull requests.

Additional aarch64 options to be solved some other time.

@jayrm jayrm merged commit ae68913 into freebasic:master Jul 14, 2019
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.

4 participants