[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-16 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:25
+
+static bool getPIE(const ArgList , const ToolChain ) {
+  if (Args.hasArg(options::OPT_static, options::OPT_shared,

I've simplified `Gnu.cpp` a bit for handling different link modes. 
ae623d16d50c9f12de7ae7ac1aa11c9d6857e081



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:51
+
+  if (IsPIE || IsStaticPIE)
+CmdArgs.push_back("-pie");

See ae623d16d50c9f12de7ae7ac1aa11c9d6857e081


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-07 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.h:61
+  bool isPICDefault() const override { return true; }
+  bool isPIEDefault(const llvm::opt::ArgList &) const override { return true; }
+  bool isPICDefaultForced() const override { return false; }

Test that the -cc1 line contains `"-pic-level" "2" "-pic-is-pie"`



Comment at: clang/lib/Driver/ToolChains/Serenity.h:68
+  getDefaultUnwindTableLevel(const llvm::opt::ArgList ) const override {
+return UnwindTableLevel::Asynchronous;
+  }

Test that the -cc1 line contains `"-funwind-tables=2"`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-07 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/test/Driver/serenity.cpp:20
+// SERENITY_X86_64: "{{(.*[^-.0-9A-Z_a-z])?}}ld.lld"
+// SERENITY_X86_64: "-pie"
+// SERENITY_X86_64: "-dynamic-linker" "/usr/lib/Loader.so" "--eh-frame-hdr"

Prefer `-SAME:` whenever applicable

ditto below



Comment at: clang/test/Driver/serenity.cpp:159
+// DEFAULT_LIBCXX: "-z" "pack-relative-relocs"
+// DEFAULT_LIBCXX: "crt0.o" "crti.o" "crtbeginS.o"
+// DEFAULT_LIBCXX: "--push-state"

`TC.GetFilePath("crti.o")` returns a path to `crti.o` if `crti.o` is found, 
otherwise a raw `crti.o` without a patch component. The raw `crti.o` indicates 
that clang driver fails to find `crti.o` in a search path.

I think you need to change `--sysroot=` to pointer to a directory tree in 
`Inputs/` where `crti.o` can be found.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-07 Thread Andrew Kaster via Phabricator via cfe-commits 
ADKaster updated this revision to Diff 558043.
ADKaster added a comment.

test LTO with option fix


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Serenity.cpp
  clang/lib/Driver/ToolChains/Serenity.h
  clang/lib/Lex/InitHeaderSearch.cpp
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/include/c++/v1/.keep
  
clang/test/Driver/Inputs/serenity_x86_64_tree/usr/include/x86_64-pc-serenity/c++/v1/.keep
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crt0.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crt0_shared.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crti.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crtn.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/local/.keep
  clang/test/Driver/pic.c
  clang/test/Driver/save-stats.c
  clang/test/Driver/serenity.cpp

Index: clang/test/Driver/serenity.cpp
===
--- /dev/null
+++ clang/test/Driver/serenity.cpp
@@ -0,0 +1,196 @@
+// UNSUPPORTED: system-windows
+
+/// Check default header and linker paths
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot=%S/Inputs/serenity_x86_64_tree \
+// RUN:   -ccc-install-dir %S/Inputs/serenity_x86_64/usr/local/bin -resource-dir=%S/Inputs/resource_dir \
+// RUN:   2>&1 | FileCheck %s --check-prefix=PATHS_X86_64
+// PATHS_X86_64:  "-resource-dir" "[[RESOURCE:[^"]+]]"
+// PATHS_X86_64:  "-internal-isystem"
+// PATHS_X86_64-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/include/x86_64-pc-serenity/c++/v1"
+// PATHS_X86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/include/c++/v1"
+// PATHS_X86_64-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]/include"
+// PATHS_X86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/include"
+// PATHS_X86_64:  "-L
+// PATHS_X86_64-SAME: {{^}}[[SYSROOT]]/usr/lib"
+
+/// Check default linker args for each supported triple
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot= 2>&1 | FileCheck %s --check-prefix=SERENITY_X86_64
+// SERENITY_X86_64: "-cc1" "-triple" "x86_64-pc-serenity"
+// SERENITY_X86_64: "{{(.*[^-.0-9A-Z_a-z])?}}ld.lld"
+// SERENITY_X86_64: "-pie"
+// SERENITY_X86_64: "-dynamic-linker" "/usr/lib/Loader.so" "--eh-frame-hdr"
+// SERENITY_X86_64: "-o" "a.out"
+// SERENITY_X86_64: "-z" "pack-relative-relocs"
+// SERENITY_X86_64: "crt0.o" "crti.o" "crtbeginS.o"
+// SERENITY_X86_64: "-lc" "crtendS.o" "crtn.o"
+
+// RUN: %clang -### %s --target=aarch64-pc-serenity --sysroot= 2>&1 | FileCheck %s --check-prefix=SERENITY_AARCH64
+// SERENITY_AARCH64: "-cc1" "-triple" "aarch64-pc-serenity"
+// SERENITY_AARCH64: "{{(.*[^-.0-9A-Z_a-z])?}}ld.lld"
+// SERENITY_AARCH64: "-pie"
+// SERENITY_AARCH64: "-dynamic-linker" "/usr/lib/Loader.so" "--eh-frame-hdr"
+// SERENITY_AARCH64: "-o" "a.out"
+// SERENITY_AARCH64: "-z" "pack-relative-relocs"
+// SERENITY_AARCH64: "crt0.o" "crti.o" "crtbeginS.o"
+// SERENITY_AARCH64: "-lc" "crtendS.o" "crtn.o"
+
+// RUN: %clang -### %s --target=riscv64-pc-serenity --sysroot= 2>&1 | FileCheck %s --check-prefix=SERENITY_RISCV64
+// SERENITY_RISCV64: "-cc1" "-triple" "riscv64-pc-serenity"
+// SERENITY_RISCV64: "{{(.*[^-.0-9A-Z_a-z])?}}ld.lld"
+// SERENITY_RISCV64: "-pie"
+// SERENITY_RISCV64: "-dynamic-linker" "/usr/lib/Loader.so" "--eh-frame-hdr"
+// SERENITY_RISCV64: "-o" "a.out"
+// SERENITY_RISCV64: "-z" "pack-relative-relocs"
+// SERENITY_RISCV64: "crt0.o" "crti.o" "crtbeginS.o"
+// SERENITY_RISCV64: "-lc" "crtendS.o" "crtn.o"
+
+/// -static-pie suppresses -dynamic-linker
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot= \
+//  -static-pie 2>&1 | FileCheck %s --check-prefix=STATIC_PIE
+// STATIC_PIE: "-pie" "-static"
+// STATIC_PIE-NOT: "-dynamic-linker"
+// STATIC_PIE: "--no-dynamic-linker" "-z" "text"
+// STATIC_PIE:  "--eh-frame-hdr" "-z" "pack-relative-relocs"
+// STATIC_PIE: "crt0.o" "crti.o" "crtbeginS.o"
+// STATIC_PIE: "-lc" "crtendS.o" "crtn.o"
+
+/// -shared forces use of shared crt files
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot= \
+//  -shared 2>&1 | FileCheck %s --check-prefix=SHARED
+// SHARED: "-shared"
+// SHARED:  "--eh-frame-hdr" "-z" "pack-relative-relocs"
+// SHARED: "crt0_shared.o" "crti.o" "crtbeginS.o"
+// SHARED: "-lc" "crtendS.o" "crtn.o"
+
+/// -static forces use of static crt files
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot= \
+//  -static 2>&1 | FileCheck %s --check-prefix=STATIC
+// STATIC: "-static"
+// STATIC:  "--eh-frame-hdr" "-z" "pack-relative-relocs"
+// STATIC: "crt0.o" "crti.o" "crtbegin.o"
+// STATIC: "-lc" "crtend.o" "crtn.o"
+
+/// -rdynamic passes -export-dynamic
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot= \
+//

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-06 Thread Brad Smith via Phabricator via cfe-commits 
brad added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:110
+assert(!Inputs.empty() && "Must have at least one input.");
+addLTOOptions(TC, Args, CmdArgs, Output, Inputs[0],
+  D.getLTOMode() == LTOK_Thin);

https://github.com/llvm/llvm-project/commit/1881832994840baa6e42f908b8822ce4d15ab632


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-05 Thread Andrew Kaster via Phabricator via cfe-commits 
ADKaster updated this revision to Diff 558012.
ADKaster added a comment.

Add more tests and remove items per comments

More tests for crt*, eh-frame-hdr, stdlib arguments
remove /usr/local/include
remove -fno-use-init-array
claim stdlib= args
remove -nopie

I hope that the new tests are more robust, but I could be missing something


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Serenity.cpp
  clang/lib/Driver/ToolChains/Serenity.h
  clang/lib/Lex/InitHeaderSearch.cpp
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/include/c++/v1/.keep
  
clang/test/Driver/Inputs/serenity_x86_64_tree/usr/include/x86_64-pc-serenity/c++/v1/.keep
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crt0.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crt0_shared.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crti.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crtn.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/local/.keep
  clang/test/Driver/pic.c
  clang/test/Driver/serenity.cpp

Index: clang/test/Driver/serenity.cpp
===
--- /dev/null
+++ clang/test/Driver/serenity.cpp
@@ -0,0 +1,196 @@
+// UNSUPPORTED: system-windows
+
+/// Check default header and linker paths
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot=%S/Inputs/serenity_x86_64_tree \
+// RUN:   -ccc-install-dir %S/Inputs/serenity_x86_64/usr/local/bin -resource-dir=%S/Inputs/resource_dir \
+// RUN:   2>&1 | FileCheck %s --check-prefix=PATHS_X86_64
+// PATHS_X86_64:  "-resource-dir" "[[RESOURCE:[^"]+]]"
+// PATHS_X86_64:  "-internal-isystem"
+// PATHS_X86_64-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/include/x86_64-pc-serenity/c++/v1"
+// PATHS_X86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/include/c++/v1"
+// PATHS_X86_64-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]/include"
+// PATHS_X86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/include"
+// PATHS_X86_64:  "-L
+// PATHS_X86_64-SAME: {{^}}[[SYSROOT]]/usr/lib"
+
+/// Check default linker args for each supported triple
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot= 2>&1 | FileCheck %s --check-prefix=SERENITY_X86_64
+// SERENITY_X86_64: "-cc1" "-triple" "x86_64-pc-serenity"
+// SERENITY_X86_64: "{{(.*[^-.0-9A-Z_a-z])?}}ld.lld"
+// SERENITY_X86_64: "-pie"
+// SERENITY_X86_64: "-dynamic-linker" "/usr/lib/Loader.so" "--eh-frame-hdr"
+// SERENITY_X86_64: "-o" "a.out"
+// SERENITY_X86_64: "-z" "pack-relative-relocs"
+// SERENITY_X86_64: "crt0.o" "crti.o" "crtbeginS.o"
+// SERENITY_X86_64: "-lc" "crtendS.o" "crtn.o"
+
+// RUN: %clang -### %s --target=aarch64-pc-serenity --sysroot= 2>&1 | FileCheck %s --check-prefix=SERENITY_AARCH64
+// SERENITY_AARCH64: "-cc1" "-triple" "aarch64-pc-serenity"
+// SERENITY_AARCH64: "{{(.*[^-.0-9A-Z_a-z])?}}ld.lld"
+// SERENITY_AARCH64: "-pie"
+// SERENITY_AARCH64: "-dynamic-linker" "/usr/lib/Loader.so" "--eh-frame-hdr"
+// SERENITY_AARCH64: "-o" "a.out"
+// SERENITY_AARCH64: "-z" "pack-relative-relocs"
+// SERENITY_AARCH64: "crt0.o" "crti.o" "crtbeginS.o"
+// SERENITY_AARCH64: "-lc" "crtendS.o" "crtn.o"
+
+// RUN: %clang -### %s --target=riscv64-pc-serenity --sysroot= 2>&1 | FileCheck %s --check-prefix=SERENITY_RISCV64
+// SERENITY_RISCV64: "-cc1" "-triple" "riscv64-pc-serenity"
+// SERENITY_RISCV64: "{{(.*[^-.0-9A-Z_a-z])?}}ld.lld"
+// SERENITY_RISCV64: "-pie"
+// SERENITY_RISCV64: "-dynamic-linker" "/usr/lib/Loader.so" "--eh-frame-hdr"
+// SERENITY_RISCV64: "-o" "a.out"
+// SERENITY_RISCV64: "-z" "pack-relative-relocs"
+// SERENITY_RISCV64: "crt0.o" "crti.o" "crtbeginS.o"
+// SERENITY_RISCV64: "-lc" "crtendS.o" "crtn.o"
+
+/// -static-pie suppresses -dynamic-linker
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot= \
+//  -static-pie 2>&1 | FileCheck %s --check-prefix=STATIC_PIE
+// STATIC_PIE: "-pie" "-static"
+// STATIC_PIE-NOT: "-dynamic-linker"
+// STATIC_PIE: "--no-dynamic-linker" "-z" "text"
+// STATIC_PIE:  "--eh-frame-hdr" "-z" "pack-relative-relocs"
+// STATIC_PIE: "crt0.o" "crti.o" "crtbeginS.o"
+// STATIC_PIE: "-lc" "crtendS.o" "crtn.o"
+
+/// -shared forces use of shared crt files
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot= \
+//  -shared 2>&1 | FileCheck %s --check-prefix=SHARED
+// SHARED: "-shared"
+// SHARED:  "--eh-frame-hdr" "-z" "pack-relative-relocs"
+// SHARED: "crt0_shared.o" "crti.o" "crtbeginS.o"
+// SHARED: "-lc" "crtendS.o" "crtn.o"
+
+/// -static forces use of static crt files
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot= \
+//  -static 2>&1 | FileCheck %s --check-prefix=STATIC
+// STATIC: "-static"
+// STATIC:  "--eh-frame-hdr" "-z"

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-05 Thread Andrew Kaster via Phabricator via cfe-commits 
ADKaster added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:76
+  if (!IsStatic || IsStaticPIE)
+CmdArgs.push_back("--eh-frame-hdr");
+

MaskRay wrote:
> ADKaster wrote:
> > MaskRay wrote:
> > > This is not tested
> > Hm. this also seems like incorrect logic. In my next push I will remove 
> > this condition around --eh-frame-hdr to match the other ToolChains.
> https://maskray.me/blog/2020-11-08-stack-unwinding I have some notes on 
> ".eh_frame_hdr and PT_GNU_EH_FRAME". 
> 
> > Clang and GCC usually pass --eh-frame-hdr to ld, with the exception that 
> > gcc -static does not pass --eh-frame-hdr. The difference is a historical 
> > choice related to `__register_frame_info`.
Ah, this explains where @BertalanD got the original logic from, as we had a gcc 
port first. It was probably copied from our patched gcc spec files.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-05 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:76
+  if (!IsStatic || IsStaticPIE)
+CmdArgs.push_back("--eh-frame-hdr");
+

ADKaster wrote:
> MaskRay wrote:
> > This is not tested
> Hm. this also seems like incorrect logic. In my next push I will remove this 
> condition around --eh-frame-hdr to match the other ToolChains.
https://maskray.me/blog/2020-11-08-stack-unwinding I have some notes on 
".eh_frame_hdr and PT_GNU_EH_FRAME". 

> Clang and GCC usually pass --eh-frame-hdr to ld, with the exception that gcc 
> -static does not pass --eh-frame-hdr. The difference is a historical choice 
> related to `__register_frame_info`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-05 Thread Andrew Kaster via Phabricator via cfe-commits 
ADKaster added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:76
+  if (!IsStatic || IsStaticPIE)
+CmdArgs.push_back("--eh-frame-hdr");
+

MaskRay wrote:
> This is not tested
Hm. this also seems like incorrect logic. In my next push I will remove this 
condition around --eh-frame-hdr to match the other ToolChains.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-05 Thread Andrew Kaster via Phabricator via cfe-commits 
ADKaster added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:202
+  addSystemInclude(DriverArgs, CC1Args,
+   concat(D.SysRoot, "/usr/local/include"));
+  addSystemInclude(DriverArgs, CC1Args, concat(D.SysRoot, "/usr/include"));

brad wrote:
> IMO if the library path is removed then the header path should be as well.
Fair. /usr/local is the prefix that all ports in the Ports/ tree are installed 
into by default (though some end up with files in /opt). We can for sure work a 
bit harder to make those headers/libs available to ports in our build 
infrastructure for them rather than putting that logic in the compiler.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:211
+  options::OPT_fno_use_init_array, true))
+CC1Args.push_back("-fno-use-init-array");
+}

MaskRay wrote:
> This is for systems that historically support .ctors/.dtors 
> https://maskray.me/blog/2021-11-07-init-ctors-init-array
> 
> If Serenity doesn't, this should be removed.
It's my understanding that we don't currently support .ctors/.dtors, though we 
did a few years ago. It is a bit confusing to me how those are related to 
.init/.fini though. We have some stubs here for `crti.S` and `crtn.S` 
https://github.com/SerenityOS/serenity/blob/cf3c8a216be5aa496844aadb43ca05ad5c47bb46/Userland/Libraries/LibC/arch/x86_64/crti.S
 which end up giving every .so and executable a DT_INIT section, but all the 
actual global ctors and dtors end up in .init_array/.fini_array.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-02 Thread Brad Smith via Phabricator via cfe-commits 
brad added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:147
+  }
+
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs,

Add in here..

```
// Silence warnings when linking C code with a C++ '-stdlib' argument.
Args.ClaimAllArgs(options::OPT_stdlib_EQ);
```

https://github.com/llvm/llvm-project/commit/760658c118f491985ec00f62f5a261293db67b95


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-02 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:164
+if (crtend_path.empty()) {
+  const char *crtend = (IsShared || IsPIE) ? "crtendS.o" : "crtend.o";
+  crtend_path = TC.GetFilePath(crtend);

crt* files are not tested


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-02 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/test/Driver/serenity.cpp:2
+// UNSUPPORTED: system-windows
+
+/// Test a cross compiler.

https://github.com/MaskRay/Config/wiki/LLVM#clanglibdriver

The test fails in a 
`-DCLANG_DEFAULT_RTLIB=compiler-rt -DCLANG_DEFAULT_UNWINDLIB=libunwind 
-DCLANG_DEFAULT_CXX_STDLIB=libc++ ` build



Comment at: clang/test/Driver/serenity.cpp:35
+// RELOCATABLE-SAME: {{^}}[[SYSROOT]]/usr/lib"
+// RELOCATABLE-NOT:  "-l
+// RELOCATABLE-NOT:  crt{{[^./]+}}.o

https://github.com/MaskRay/Config/wiki/LLVM#clanglibdriver

The `NOT -l`test fails in a 
`-DCLANG_DEFAULT_RTLIB=compiler-rt -DCLANG_DEFAULT_UNWINDLIB=libunwind 
-DCLANG_DEFAULT_CXX_STDLIB=libc++ ` build


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-02 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:31
+  Arg *Last = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,
+  options::OPT_nopie);
+  return Last ? Last->getOption().matches(options::OPT_pie) : true;

`-no-pie` is nowadays canonical and GCC does't support `-nopie`. You can drop 
`-nopie`.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:76
+  if (!IsStatic || IsStaticPIE)
+CmdArgs.push_back("--eh-frame-hdr");
+

This is not tested



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:85
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("pack-relative-relocs");
+

This is untested



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:211
+  options::OPT_fno_use_init_array, true))
+CC1Args.push_back("-fno-use-init-array");
+}

This is for systems that historically support .ctors/.dtors 
https://maskray.me/blog/2021-11-07-init-ctors-init-array

If Serenity doesn't, this should be removed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-02 Thread Brad Smith via Phabricator via cfe-commits 
brad added a comment.

But the rest LGTM as a start.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-11-02 Thread Brad Smith via Phabricator via cfe-commits 
brad added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:202
+  addSystemInclude(DriverArgs, CC1Args,
+   concat(D.SysRoot, "/usr/local/include"));
+  addSystemInclude(DriverArgs, CC1Args, concat(D.SysRoot, "/usr/include"));

IMO if the library path is removed then the header path should be as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-10-31 Thread Andrew Kaster via Phabricator via cfe-commits 
ADKaster updated this revision to Diff 557945.
ADKaster added a comment.

Brad's comments, and clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Serenity.cpp
  clang/lib/Driver/ToolChains/Serenity.h
  clang/lib/Lex/InitHeaderSearch.cpp
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/include/c++/v1/.keep
  
clang/test/Driver/Inputs/serenity_x86_64_tree/usr/include/x86_64-pc-serenity/c++/v1/.keep
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crt0.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crt0_shared.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crti.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crtn.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/local/.keep
  clang/test/Driver/serenity.cpp

Index: clang/test/Driver/serenity.cpp
===
--- /dev/null
+++ clang/test/Driver/serenity.cpp
@@ -0,0 +1,36 @@
+// UNSUPPORTED: system-windows
+
+/// Test a cross compiler.
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot=%S/Inputs/serenity_x86_64_tree \
+// RUN:   -ccc-install-dir %S/Inputs/serenity_x86_64/usr/local/bin -resource-dir=%S/Inputs/resource_dir \
+// RUN:   --stdlib=platform --rtlib=platform --unwindlib=platform 2>&1 | FileCheck %s --check-prefix=SERENITY_x86_64
+// SERENITY_x86_64:  "-resource-dir" "[[RESOURCE:[^"]+]]"
+// SERENITY_x86_64:  "-internal-isystem"
+// SERENITY_x86_64-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/include/x86_64-pc-serenity/c++/v1"
+// SERENITY_x86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/include/c++/v1"
+// SERENITY_x86_64-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]/include"
+// SERENITY_x86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/local/include"
+// SERENITY_x86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/include"
+// SERENITY_x86_64:  "-L
+// SERENITY_x86_64-SAME: {{^}}[[SYSROOT]]/usr/lib"
+
+/// Loader name is the same for all architectures
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot= \
+// RUN:   --stdlib=platform --rtlib=platform --unwindlib=platform 2>&1 | FileCheck %s --check-prefix=DYNAMIC_LOADER
+// RUN: %clang -### %s --target=aarch64-pc-serenity --sysroot= \
+// RUN:   --stdlib=platform --rtlib=platform --unwindlib=platform 2>&1 | FileCheck %s --check-prefix=DYNAMIC_LOADER
+// RUN: %clang -### %s --target=riscv64-pc-serenity --sysroot= \
+// RUN:   --stdlib=platform --rtlib=platform --unwindlib=platform 2>&1 | FileCheck %s --check-prefix=DYNAMIC_LOADER
+// DYNAMIC_LOADER: "-dynamic-linker" "/usr/lib/Loader.so"
+
+/// -r suppresses -dynamic-linker, default -l, and crt*.o like -nostdlib.
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot=%S/Inputs/serenity_x86_64_tree \
+// RUN:   -ccc-install-dir %S/Inputs/serenity_x86_64_tree/usr/local/bin -resource-dir=%S/Inputs/resource_dir \
+// RUN:   --stdlib=platform --rtlib=platform -r 2>&1 | FileCheck %s --check-prefix=RELOCATABLE
+// RELOCATABLE-NOT:  "-dynamic-linker"
+// RELOCATABLE:  "-internal-isystem"
+// RELOCATABLE-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/include/x86_64-pc-serenity/c++/v1"
+// RELOCATABLE:  "-L
+// RELOCATABLE-SAME: {{^}}[[SYSROOT]]/usr/lib"
+// RELOCATABLE-NOT:  "-l
+// RELOCATABLE-NOT:  crt{{[^./]+}}.o
Index: clang/lib/Lex/InitHeaderSearch.cpp
===
--- clang/lib/Lex/InitHeaderSearch.cpp
+++ clang/lib/Lex/InitHeaderSearch.cpp
@@ -304,6 +304,7 @@
   case llvm::Triple::PS4:
   case llvm::Triple::PS5:
   case llvm::Triple::RTEMS:
+  case llvm::Triple::Serenity:
   case llvm::Triple::Solaris:
   case llvm::Triple::WASI:
   case llvm::Triple::ZOS:
Index: clang/lib/Driver/ToolChains/Serenity.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/Serenity.h
@@ -0,0 +1,89 @@
+//=== Serenity.h - SerenityOS ToolChain Implementation --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_SERENITY_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_SERENITY_H
+
+#include "Gnu.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace tools {
+namespace serenity {
+
+class LLVM_LIBRARY_VISIBILITY Linker final : public Tool {
+public:
+  Linker(const ToolChain ) :

[PATCH] D154396: [clang] Add support for SerenityOS

2023-10-29 Thread Brad Smith via Phabricator via cfe-commits 
brad added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.h:81
+
+  unsigned GetDefaultDwarfVersion() const override { return 5; }
+

This just overrides the default DWARF version anyway.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-10-29 Thread Brad Smith via Phabricator via cfe-commits 
brad added a comment.

You also have to add Serenity to clang/lib/Lex/InitHeaderSearch.cpp 
InitHeaderSearch::ShouldAddDefaultIncludePaths().


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-10-29 Thread Brad Smith via Phabricator via cfe-commits 
brad added a comment.

Make use of the concat macro with the various header and library paths.
https://github.com/llvm/llvm-project/commit/1d0cc510516d50c459f78896a0375fadb13a2b45




Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:78
+
+  if (Output.isFilename()) {
+CmdArgs.push_back("-o");

Like the other ToolChains this should have before it..

assert((Output.isFilename() || Output.isNothing()) && "Invalid output.");



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:107
+  Args.AddAllArgs(CmdArgs, options::OPT_u);
+
+  TC.AddFilePathLibArgs(Args, CmdArgs);

https://github.com/llvm/llvm-project/commit/894927b491b7c62917ffa7ad665841683095317c



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:119
+  Args.AddAllArgs(CmdArgs, options::OPT_t);
+  Args.AddAllArgs(CmdArgs, options::OPT_r);
+

https://github.com/llvm/llvm-project/commit/894927b491b7c62917ffa7ad665841683095317c


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-10-29 Thread Andrew Kaster via Phabricator via cfe-commits 
ADKaster added a comment.

In D154396#4655496 , @brad wrote:

> In D154396#4655494 , @ADKaster 
> wrote:
>
>> @MaskRay @phosek Daniel and I have updated the patch set,  Would you rather 
>> I update the phab patch series, or re-upload the set as GitHub PRs?
>
> Update the phab patch series.

Sounds good, done. Some of the patches didn't need updates, and others needed a 
rebase. I already abandoned the libcxx patch as the project still needs to 
figure out the proper way to provision a machine to use as a libcxx buildkite 
runner (and where to fund it from :) ). We'll resubmit that one on GitHub after 
these are in and we get that sorted out.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-10-29 Thread Andrew Kaster via Phabricator via cfe-commits 
ADKaster updated this revision to Diff 557927.
ADKaster added a comment.
Herald added subscribers: wangpc, s.egerton, ormris, simoncook, asb.

Base on Generic_ELF driver, add tests, add myself as co-author


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Serenity.cpp
  clang/lib/Driver/ToolChains/Serenity.h
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/include/c++/v1/.keep
  
clang/test/Driver/Inputs/serenity_x86_64_tree/usr/include/x86_64-pc-serenity/c++/v1/.keep
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crt0.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crt0_shared.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crti.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/lib/crtn.o
  clang/test/Driver/Inputs/serenity_x86_64_tree/usr/local/.keep
  clang/test/Driver/serenity.cpp

Index: clang/test/Driver/serenity.cpp
===
--- /dev/null
+++ clang/test/Driver/serenity.cpp
@@ -0,0 +1,36 @@
+// UNSUPPORTED: system-windows
+
+/// Test a cross compiler.
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot=%S/Inputs/serenity_x86_64_tree \
+// RUN:   -ccc-install-dir %S/Inputs/serenity_x86_64/usr/local/bin -resource-dir=%S/Inputs/resource_dir \
+// RUN:   --stdlib=platform --rtlib=platform --unwindlib=platform 2>&1 | FileCheck %s --check-prefix=SERENITY_x86_64
+// SERENITY_x86_64:  "-resource-dir" "[[RESOURCE:[^"]+]]"
+// SERENITY_x86_64:  "-internal-isystem"
+// SERENITY_x86_64-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/include/x86_64-pc-serenity/c++/v1"
+// SERENITY_x86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/include/c++/v1"
+// SERENITY_x86_64-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]/include"
+// SERENITY_x86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/local/include"
+// SERENITY_x86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/include"
+// SERENITY_x86_64:  "-L
+// SERENITY_x86_64-SAME: {{^}}[[SYSROOT]]/usr/lib"
+
+/// Loader name is the same for all architectures
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot= \
+// RUN:   --stdlib=platform --rtlib=platform --unwindlib=platform 2>&1 | FileCheck %s --check-prefix=DYNAMIC_LOADER
+// RUN: %clang -### %s --target=aarch64-pc-serenity --sysroot= \
+// RUN:   --stdlib=platform --rtlib=platform --unwindlib=platform 2>&1 | FileCheck %s --check-prefix=DYNAMIC_LOADER
+// RUN: %clang -### %s --target=riscv64-pc-serenity --sysroot= \
+// RUN:   --stdlib=platform --rtlib=platform --unwindlib=platform 2>&1 | FileCheck %s --check-prefix=DYNAMIC_LOADER
+// DYNAMIC_LOADER: "-dynamic-linker" "/usr/lib/Loader.so"
+
+/// -r suppresses -dynamic-linker, default -l, and crt*.o like -nostdlib.
+// RUN: %clang -### %s --target=x86_64-pc-serenity --sysroot=%S/Inputs/serenity_x86_64_tree \
+// RUN:   -ccc-install-dir %S/Inputs/serenity_x86_64_tree/usr/local/bin -resource-dir=%S/Inputs/resource_dir \
+// RUN:   --stdlib=platform --rtlib=platform -r 2>&1 | FileCheck %s --check-prefix=RELOCATABLE
+// RELOCATABLE-NOT:  "-dynamic-linker"
+// RELOCATABLE:  "-internal-isystem"
+// RELOCATABLE-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/include/x86_64-pc-serenity/c++/v1"
+// RELOCATABLE:  "-L
+// RELOCATABLE-SAME: {{^}}[[SYSROOT]]/usr/lib"
+// RELOCATABLE-NOT:  "-l
+// RELOCATABLE-NOT:  crt{{[^./]+}}.o
Index: clang/lib/Driver/ToolChains/Serenity.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/Serenity.h
@@ -0,0 +1,91 @@
+//=== Serenity.h - SerenityOS ToolChain Implementation --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_SERENITY_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_SERENITY_H
+
+#include "Gnu.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace tools {
+namespace serenity {
+
+class LLVM_LIBRARY_VISIBILITY Linker final : public Tool {
+public:
+  Linker(const ToolChain ) : Tool("serenity::Linker", "linker", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+  bool isLinkJob() const override { return true; }
+
+  void ConstructJob(Compilation , const JobAction ,
+const InputInfo , const InputInfoList ,
+const llvm::opt::ArgList ,
+const char *LinkingOutput)

[PATCH] D154396: [clang] Add support for SerenityOS

2023-10-29 Thread Brad Smith via Phabricator via cfe-commits 
brad added a comment.

In D154396#4655494 , @ADKaster wrote:

> @MaskRay @phosek Daniel and I have updated the patch set,  Would you rather I 
> update the phab patch series, or re-upload the set as GitHub PRs?

Update the phab patch series.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-10-29 Thread Andrew Kaster via Phabricator via cfe-commits 
ADKaster added a comment.

@MaskRay @phosek Daniel and I have updated the patch set,  Would you rather I 
update the phab patch series, or re-upload the set as GitHub PRs?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-07-08 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:85
+  auto linkerIs = [Exec](const char *name) {
+return llvm::sys::path::filename(Exec).equals_insensitive(name) ||
+   llvm::sys::path::stem(Exec).equals_insensitive(name);

ADKaster wrote:
> MaskRay wrote:
> > Avoid `equals_insensitive`. It's only recommended in some places for 
> > Windows.
> What is the recommendation for this use case instead? This is the same 
> pattern that is used by the Fuchsia driver. 
> https://github.com/llvm/llvm-project/blob/01e3393b94d194ee99e57f23f9c08160cef94753/clang/lib/Driver/ToolChains/Fuchsia.cpp#L59-L61
> 
> though it looks like Fuchsia used that pattern back when it was called 
> `equals_lower()`. 
> https://github.com/llvm/llvm-project/commit/3e199ecdadf7b546054c5a5820d1678f1e83c821
I think Fuchsia should not use `equals_lower`. There is never a case that we'd 
use something like `ld.LLD`.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:127
+  Args.AddAllArgs(CmdArgs, options::OPT_T_Group);
+  Args.AddAllArgs(CmdArgs, options::OPT_e);
+  Args.AddAllArgs(CmdArgs, options::OPT_s);

`OPT_e` has the LinkerInput flag and is rendered by AddLinkerInputs. Remove 
`Args.AddAllArgs(CmdArgs, options::OPT_e);` to avoid duplicate `-e`.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:130
+  Args.AddAllArgs(CmdArgs, options::OPT_t);
+  Args.AddAllArgs(CmdArgs, options::OPT_Z_Flag);
+  Args.AddAllArgs(CmdArgs, options::OPT_r);

gcc doesn't recognize `-Z`. I think it is an early (200x) mistake that some 
targets render `-Z`. Drop it.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:143
+const SanitizerArgs  = TC.getSanitizerArgs(Args);
+if (Sanitize.needsUbsanRt())
+  CmdArgs.push_back("-lubsan");

I wonder why you don't use the common `addSanitizerRuntimes`. ubsan runtime is 
normally `libclang_rt.ubsan_standalone*`



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:195
+getProgramPaths().push_back(getDriver().Dir);
+  getFilePaths().push_back(getDriver().SysRoot + "/usr/local/lib");
+  getFilePaths().push_back(getDriver().SysRoot + "/usr/lib");

`/usr/local/lib` should not be in the default search path.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-07-07 Thread Andrew Kaster via Phabricator via cfe-commits 
ADKaster planned changes to this revision.
ADKaster added a comment.

Planning to rebase on top of Generic_ELF per Petr's suggestion.




Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:85
+  auto linkerIs = [Exec](const char *name) {
+return llvm::sys::path::filename(Exec).equals_insensitive(name) ||
+   llvm::sys::path::stem(Exec).equals_insensitive(name);

MaskRay wrote:
> Avoid `equals_insensitive`. It's only recommended in some places for Windows.
What is the recommendation for this use case instead? This is the same pattern 
that is used by the Fuchsia driver. 
https://github.com/llvm/llvm-project/blob/01e3393b94d194ee99e57f23f9c08160cef94753/clang/lib/Driver/ToolChains/Fuchsia.cpp#L59-L61

though it looks like Fuchsia used that pattern back when it was called 
`equals_lower()`. 
https://github.com/llvm/llvm-project/commit/3e199ecdadf7b546054c5a5820d1678f1e83c821


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-07-05 Thread Petr Hosek via Phabricator via cfe-commits 
phosek added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.h:38
+
+class LLVM_LIBRARY_VISIBILITY Serenity : public ToolChain {
+public:

Have you considered inheriting from `Generic_GCC` to allow more code reuse?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-07-05 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Serenity.cpp:85
+  auto linkerIs = [Exec](const char *name) {
+return llvm::sys::path::filename(Exec).equals_insensitive(name) ||
+   llvm::sys::path::stem(Exec).equals_insensitive(name);

Avoid `equals_insensitive`. It's only recommended in some places for Windows.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-07-05 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

Upstreaming support requires rigid testing, otherwise it's fairly easy for 
other contributors to break your code while refactoring.
https://maskray.me/blog/2021-08-08-toolchain-testing "I don't know whether a 
test is needed"

`clang/test/Driver` has many tests that could have been written in a better 
way. There are some good ones, e.g. `linux-cross.cpp`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154396/new/

https://reviews.llvm.org/D154396

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154396: [clang] Add support for SerenityOS

2023-07-03 Thread Andrew Kaster via Phabricator via cfe-commits 
ADKaster created this revision.
Herald added a project: All.
ADKaster requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Adds support for the `$arch-pc-serenity` target to the Clang front end.
This makes the compiler look for libraries and headers in the right
places, and enables some security mitigations like stack-smashing
protection and position-independent code by default.

Depends on D154395 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D154396

Files:
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Serenity.cpp
  clang/lib/Driver/ToolChains/Serenity.h

Index: clang/lib/Driver/ToolChains/Serenity.h
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/Serenity.h
@@ -0,0 +1,100 @@
+//=== Serenity.h - SerenityOS ToolChain Implementation --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_SERENITY_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_SERENITY_H
+
+#include "clang/Basic/LangOptions.h"
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
+
+namespace clang {
+namespace driver {
+namespace tools {
+namespace serenity {
+
+class LLVM_LIBRARY_VISIBILITY Linker : public Tool {
+public:
+  Linker(const ToolChain ) : Tool("serenity::Linker", "linker", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+  bool isLinkJob() const override { return true; }
+
+  void ConstructJob(Compilation , const JobAction ,
+const InputInfo , const InputInfoList ,
+const llvm::opt::ArgList ,
+const char *LinkingOutput) const override;
+};
+} // end namespace serenity
+} // end namespace tools
+
+namespace toolchains {
+
+class LLVM_LIBRARY_VISIBILITY Serenity : public ToolChain {
+public:
+  Serenity(const Driver , const llvm::Triple ,
+   const llvm::opt::ArgList );
+
+  void
+  AddClangSystemIncludeArgs(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ) const override;
+
+  void AddClangCXXStdlibIncludeArgs(
+  const llvm::opt::ArgList ,
+  llvm::opt::ArgStringList ) const override;
+
+  RuntimeLibType GetDefaultRuntimeLibType() const override {
+return ToolChain::RLT_CompilerRT;
+  }
+
+  CXXStdlibType GetDefaultCXXStdlibType() const override {
+return ToolChain::CST_Libcxx;
+  }
+
+  UnwindLibType GetUnwindLibType(const llvm::opt::ArgList ) const override;
+
+  void
+  addClangTargetOptions(const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ,
+Action::OffloadKind DeviceOffloadKind) const override;
+
+  const char *getDefaultLinker() const override { return "ld.lld"; }
+
+  bool HasNativeLLVMSupport() const override { return true; }
+
+  bool IsIntegratedAssemblerDefault() const override { return true; }
+
+  bool isPICDefault() const override { return true; }
+  bool isPIEDefault(const llvm::opt::ArgList &) const override { return false; }
+  bool isPICDefaultForced() const override { return false; }
+
+  bool IsMathErrnoDefault() const override { return false; }
+
+  UnwindTableLevel
+  getDefaultUnwindTableLevel(const llvm::opt::ArgList ) const override {
+return UnwindTableLevel::Asynchronous;
+  }
+
+  bool useRelaxRelocations() const override { return true; }
+
+  LangOptions::StackProtectorMode
+  GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
+return LangOptions::SSPStrong;
+  }
+
+  unsigned GetDefaultDwarfVersion() const override { return 5; }
+
+protected:
+  Tool *buildLinker() const override;
+};
+
+} // end namespace toolchains
+} // end namespace driver
+} // end namespace clang
+
+#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_SERENITY_H
Index: clang/lib/Driver/ToolChains/Serenity.cpp
===
--- /dev/null
+++ clang/lib/Driver/ToolChains/Serenity.cpp
@@ -0,0 +1,336 @@
+//=== Serenity.cpp - SerenityOS ToolChain Implementation *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Serenity.h"
+#include "CommonArgs.h"
+#include "clang/Config/config.h"
+#include "clang/Driver/Compilation.h"
+#include "clang/Driver/Driver.h"
+#include