User Details
- User Since
- Dec 30 2016, 3:24 PM (325 w, 1 d)
- Roles
- Administrator
Yesterday
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
Thank you!
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.
Thanks!
Tue, Mar 14
Is this worth a unit test?
Mon, Mar 13
LGTM
While --no-undefined is an alias for -z defs, be consistent in the subject and the body.
Thanks!
Sun, Mar 12
Sat, Mar 11
Fri, Mar 10
Fix -gdwarf-aranges
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.