- User Since
- Dec 30 2016, 3:24 PM (325 w, 1 d)
There have been some changes in the relocation area. Is this patch unneeded now?
Fri, Mar 24
(back from a long vacation with little time spending on reviews.llvm.org )
Thu, Mar 23
Thank you for splitting! I am just back from a long vacation to check some emails and will start another long vacation soon (03/29)🤣 It will likely take some time for me to read through this change:)
Adjust some terms.
This change is correct for Linux. llvm/CMakeLists.txt says:
if(CMAKE_SYSTEM_NAME MATCHES "BSD|Linux|OS390") set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default ON)
Wed, Mar 22
The unknown_ext_version.s case is common (for GCC uses) and will get improved.
Tue, Mar 21
No objection. LG once @jrtc27 is happy.
"Close" seems more appropriate to me as this isn't a bug (previously not an intended use case).
Sorry for my belated response as I was on a trip for quite a few days.
LG! llvm/test/CodeGen/Mips/ tests are unrelated to the driver change and the part should be committed separately.
Mon, Mar 20
Do you have a use case for the more flexible function signature?
Sat, Mar 18
for current GNU Binutils releases.
Tue, Mar 14
Is this worth a unit test?
Mon, Mar 13
While --no-undefined is an alias for -z defs, be consistent in the subject and the body.
Sun, Mar 12
Sat, Mar 11
Fri, Mar 10
computeTargetTriple has several users. You probably want to check all these code paths do not need if (const Arg *A = Args.getLastArg(options::OPT_target)).
For the uses in BuildCompilation, it seems fine.
(I'm going to be out for 3 weeks in the next 4 weeks. Happy if others are happy. Don't wait on me :) )
I have an analysis of the code path: https://reviews.llvm.org/D145791#4185609
There are some if conditions, virtual function calls, SmallVector range append overhead which cannot be eliminated.
LGTM, but make sure @RKSimon is happy as well.
Thu, Mar 9
Thanks! LGTM from my review. It'd be good to have a review from a domain expert.
Due to this we started seeing assembler errors with certain .c and .cpp files -
"Error: file number 1 already allocated"
(I'll be be out for most of the next 4 weeks and may not have much time to review patches.)
Abandoned by a revert of D118493
Yeah; I wish that we don't do this. IIRC even a RISC-V binutils maintainers said the behavior is strange but perhaps people don't have enough motivation to fix that.
We don't really need to copy the behavior as Clang Driver has passed -X.
Wed, Mar 8
At least with that this diff shows, the saving is small. I'd hope that we don't toggle -msave-restore for -Os/-Oz. Users can specify -msave-restore themselves.
Since the touched code is still in review, you need to merge the changes into the prior patches.