swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.58k stars 10.36k forks source link

[SR-7148] Compile-time conditionalization of endianness in the compiler should be done by target checking #49696

Open jckarter opened 6 years ago

jckarter commented 6 years ago
Previous ID SR-7148
Radar None
Original Reporter @jckarter
Type Bug
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug, StarterBug | |Assignee | None | |Priority | Medium | md5: 7f26b7e8c0700431c7bacc1871180f85

Issue Description:

IRGen appears to have grown compile-time conditionalizations on host properties, e.g.:

https://github.com/apple/swift/blob/master/lib/IRGen/GenEnum.cpp#L3892

This is not correct if cross-compiling for a target with different endianness than the compiler host. This should check the properties of the target instead of being a #if

jckarter commented 6 years ago

Starter bug hints: Checking endianness can be done given an IRGenModule &IGM by checking IGM.getModule()->getDataLayout().isBigEndian() or .isLittleEndian(), and bit width by IGM.SizeTy == IGM.Int64Ty or IGM.SizeTy == IGM.Int32Ty.

jckarter commented 6 years ago

s390x is a big-endian 64-bit target maintained by IBM's Swift team. You should be able to use it as a test target. You can write IRGen tests for cross-compilation by providing a //RUN: line in the test file that specifies a target triple in the command line:

// RUN: %target-swift-frontend -parse-stdlib -emit-ir -target s390x-ibm-linux %s | %FileCheck --check-prefix=BE
// RUN: %target-swift-frontend -parse-stdlib -emit-ir -target x86_64-pc-linux %s | %FileCheck --check-prefix=LE

// BE: this will be checked for in big endian builds
// LE: this will be checked for in little endian builds

Note that I also included -parse-stdlib so the compiler won't try to load the standard library for the target platform, which might not be built on all hosts. You should be able to write tests that don't use anything from Swift's standard library, though.

swift-ci commented 6 years ago

Comment by Tapan Thaker (JIRA)

The places I found occurrences of _LITTLE_ENDIAN_ or _BIG_ENDIAN_ are the following:

If I understand correctly, it should be okay to have these macros in the runtime & is a problem only in lib/IRGen

jckarter commented 6 years ago

You're correct. The compiler shouldn't change behavior based on host characteristics, but the runtime is compiled for the target(s).

belkadan commented 5 years ago

Resetting assignee on all Starter Bugs last modified in 2018.