smeenai (Shoaib Meenai)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 19 2016, 10:21 AM (109 w, 4 d)

Recent Activity

Yesterday

smeenai updated subscribers of D52506: Reset input section pointers to null on each linker invocation..
Tue, Sep 25, 11:45 AM

Fri, Sep 21

smeenai added a comment to D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`..

Would you be okay with me renaming cfstring to .cfstring for ELF so it's in line with ELF section naming conventions (I'm not entirely sure if that could cause issues with ObjC stuff)? And yes I think it's best to avoid that code-path altogether if it turns out to be a constant as that's likely from the declaration of the type, I'll write a FileCheck test in a moment and check that I can apply the same logic to ELF aside from the DLL stuff as CoreFoundation needs to export the symbol somehow while preserving the implicit extern attribute for every other TU that comes from the builtin that CFSTR expands to. Is what I'm suggesting making sense? If not, let me know, I may be misunderstanding what's happening here.

Following the ELF conventions seems like the right thing to do, assuming it doesn't cause compatibility problems. David Chisnall might know best here.

Fri, Sep 21, 1:59 PM

Thu, Sep 13

smeenai added inline comments to D52051: [LLD] [COFF] Avoid copying of chunk vectors. NFC..
Thu, Sep 13, 3:32 PM

Tue, Sep 11

smeenai accepted D51913: [libFuzzer] [Windows] Include windows.h and psapi.h with lowercase.

All right, those are all fair arguments. LGTM.

Tue, Sep 11, 10:21 AM
smeenai updated subscribers of D51913: [libFuzzer] [Windows] Include windows.h and psapi.h with lowercase.

The existing capitalization (as in before your change) is the correct one for these headers in the Windows SDK, and it's also possible to cross-compile against the Windows SDK on a case-sensitive filesystem. However, doing that cross-compilation requires using a VFS overlay or case-correcting symlinks or ciopfs or whatever anyway, because the casing in the rest of the Windows SDK is hopelessly broken, so I'm okay with this change. I'll let @rnk confirm that it's okay to change the capitalization to the non-Windows SDK form though, and I'm also CCing @zturner since he also did cross-compilation work against the Windows SDK on Linux.

Tue, Sep 11, 12:55 AM

Mon, Sep 10

smeenai accepted D51877: [compiler-rt] [Windows] Include BaseTsd.h with lowercase.

It's lowercase in the Windows SDK as well (or at least in versions 14393, 16299, and 17134), so this LGTM.

Mon, Sep 10, 1:03 PM

Wed, Sep 5

smeenai added a reviewer for D51607: [ELF] Fix bugzilla #38748 for option no-rosegment: ruiu.

You'll wanna upload the patch with full context (http://llvm.org/docs/Phabricator.html).

Wed, Sep 5, 4:32 PM · lld

Wed, Aug 29

smeenai added a comment to D51454: [LLD] [COFF] Make sections with runtime pseudo relocations writable.

You can still use VirtualProtectFromApp with UWP; you just can't do W+X.

Wed, Aug 29, 3:04 PM

Tue, Aug 28

smeenai committed rL340885: [LLDB] Fix script to work with GNU sed.
[LLDB] Fix script to work with GNU sed
Tue, Aug 28, 4:48 PM
smeenai committed rLLDB340885: [LLDB] Fix script to work with GNU sed.
[LLDB] Fix script to work with GNU sed
Tue, Aug 28, 4:48 PM
smeenai closed D51374: [LLDB] Fix script to work with GNU sed.
Tue, Aug 28, 4:48 PM
smeenai added a comment to D51373: [LLDB] Fix script to work with GNU sed.

D51374 has the mailing list. Sorry for the noise.

Tue, Aug 28, 12:18 PM
smeenai created D51374: [LLDB] Fix script to work with GNU sed.
Tue, Aug 28, 12:17 PM
smeenai abandoned D51373: [LLDB] Fix script to work with GNU sed.

Argh, why didn't Herald add the -commits list?

Tue, Aug 28, 12:17 PM
smeenai created D51373: [LLDB] Fix script to work with GNU sed.
Tue, Aug 28, 12:17 PM
smeenai updated subscribers of D49485: CMake: use new policy for CMP0051.

@beanz did this in https://reviews.llvm.org/rL340436

Tue, Aug 28, 12:04 PM

Aug 24 2018

smeenai added inline comments to D51199: CodeGen: Add two more conditions for adding symbols to the address-significance table..
Aug 24 2018, 1:16 PM

Aug 22 2018

smeenai updated subscribers of D50144: Add Windows support for the GNUstep Objective-C ABI V2..

Sorry for the belated response; I was on vacation.

Aug 22 2018, 12:15 PM

Aug 20 2018

smeenai added a reviewer for D50998: [LLD] [COFF] Check the instructions in ARM MOV32T relocations: compnerd.
Aug 20 2018, 2:54 PM

Aug 15 2018

smeenai added inline comments to D50144: Add Windows support for the GNUstep Objective-C ABI V2..
Aug 15 2018, 12:00 PM
smeenai updated subscribers of D44501: COFF: Move assignment of section offsets to assignAddresses(). NFCI..

This makes the design a little more similar to the ELF linker and should allow for features such as ARM range extension thunks to be implemented more easily.

@pcc - I found you mentioned this as part of this change; do you have any concrete plan on how to add ARM range extension thunks? I'm pondering giving that a try, but if you have ideas about how/where to start, that'd get me started quicker.

Aug 15 2018, 10:45 AM

Aug 14 2018

smeenai added a comment to D49672: [CMake] Honor LLVM_EXTERNAL_<proj>_SOURCE_DIR.

This is pretty confusing, but there's both LLVM_ENABLE_PROJECTS, which assumes the side-by-side layout, and LLVM_EXTERNAL_PROJECTS, which assumes the corresponding SOURCE_DIR variables will be set. It sounds like LLVM_EXTERNAL_PROJECTS is what you want for your use case.

Aug 14 2018, 4:16 PM

Jul 19 2018

smeenai added a comment to D49573: [CMake] Option to control whether shared/static library is installed.

Also, this ended up going to llvm-commits instead of cfe-commits, which isn't ideal.

Jul 19 2018, 4:08 PM
smeenai added a comment to D49573: [CMake] Option to control whether shared/static library is installed.

Patch is missing context.

Jul 19 2018, 4:07 PM
smeenai committed rL337485: [Analysis] Fix typo in assert. NFC.
[Analysis] Fix typo in assert. NFC
Jul 19 2018, 12:16 PM

Jul 18 2018

smeenai added a comment to D49509: [libc++] Allow running ABI list tests with different ABI versions.

This went out to llvm-commits. You may wanna re-upload with cfe-commits added instead.

Jul 18 2018, 5:52 PM
smeenai added inline comments to D48798: llvm-nm: Observe -no-llvm-bc for archive members.
Jul 18 2018, 3:30 PM

Jul 17 2018

smeenai added inline comments to D35077: [RFC] Build LLVM-C.dll on MSVC that exports only the C API.
Jul 17 2018, 2:20 PM

Jul 16 2018

smeenai added a comment to D35077: [RFC] Build LLVM-C.dll on MSVC that exports only the C API.

Sorry this has been sitting for so long. I have some questions, and I'll also try to play around with this locally because I don't fully understand some parts.

Jul 16 2018, 11:35 PM
smeenai added inline comments to D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types.
Jul 16 2018, 4:46 PM

Jul 15 2018

smeenai added inline comments to D42762: Rewrite the VS Integration Scripts.
Jul 15 2018, 11:05 PM

Jul 14 2018

smeenai accepted D49345: [CMake] Use correct variable as header install prefix.

Seems like an obvious enough fix, so I'm jumping in to LGTM.

Jul 14 2018, 8:43 PM

Jul 13 2018

smeenai accepted D49335: [CMake] Pass CMAKE_INSTALL_DO_STRIP to external projects.

LGTM

Jul 13 2018, 10:47 PM

Jul 12 2018

smeenai resigned from D49227: Override a bit fields layout from an external source.

I'm not familiar enough with this to review it.

Jul 12 2018, 4:31 PM · Restricted Project

Jul 11 2018

smeenai added a comment to D49189: Omit path to lld binary from lld's error and warning diagnostics..

Actually, I lied; bfd and gold appear to print their full path. gcc does print the basename though, and that seems like a good compromise.

Jul 11 2018, 1:32 PM
smeenai added a comment to D49189: Omit path to lld binary from lld's error and warning diagnostics..

Maybe printing out the basename of argv[0] could get the right balance. What do you think?

Jul 11 2018, 1:32 PM
smeenai added a comment to D49189: Omit path to lld binary from lld's error and warning diagnostics..

I think we should at least include "ld.lld: " part in the error message, because without that, it is sometimes hard to identify what command is emitting an error in a long build log.

Jul 11 2018, 10:56 AM

Jul 10 2018

smeenai added inline comments to D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types.
Jul 10 2018, 10:34 PM

Jul 9 2018

smeenai added inline comments to D49059: [MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified.
Jul 9 2018, 2:46 PM
smeenai added a comment to D49055: [compiler-rt] [builtins] Implement the __chkstk function for ARM for MinGW.

Microsoft's implementation also checks the stack allocation against the thread's stack limit. Do you care about doing that?

Jul 9 2018, 2:41 PM
smeenai added inline comments to D49059: [MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified.
Jul 9 2018, 2:24 PM
smeenai accepted D49059: [MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified.

LGTM, particularly given r314138.

Jul 9 2018, 2:23 PM
smeenai accepted D49054: [MinGW] Treat any -lucrt* as replacing -lmsvcrt.

LGTM

Jul 9 2018, 2:06 PM

Jun 29 2018

smeenai committed rL336034: [libc++abi] Look for __config instead of vector.
[libc++abi] Look for __config instead of vector
Jun 29 2018, 6:30 PM
smeenai committed rCXXA336034: [libc++abi] Look for __config instead of vector.
[libc++abi] Look for __config instead of vector
Jun 29 2018, 6:30 PM
smeenai committed rCXXA336032: [libc++abi] Limit libc++ header search to specified paths.
[libc++abi] Limit libc++ header search to specified paths
Jun 29 2018, 6:13 PM
smeenai committed rL336032: [libc++abi] Limit libc++ header search to specified paths.
[libc++abi] Limit libc++ header search to specified paths
Jun 29 2018, 6:09 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Jun 29 2018, 6:09 PM
smeenai added a comment to D48694: [libc++abi] Limit libc++ header search to specified paths.

For whether it makes sense to search for includes in the system path, that boils down to @ldionne's question of whether building libc++abi against a system-installed libc++ is supported. I actually don't know the answer to that myself ... @dexonsmith and @EricWF, what are your thoughts on that? The current search won't find the system-installed libc++ headers on Darwin anyway though, where they're placed in the compiler's include directory rather than a system include directory.

Building libc++abi against an installed libc++ doesn't make sense IMO, so I don't care if we try to support it. Libc++abi is a lower lever component with libc++ being built on top of it.
If you want to update libc++abi, you should be building a new libc++ as well. (Note: Using system libc++abi headers to build libc++ would make sense though).

As an aside, libc++abi should really live in the libc++ repository. That way it would always have the correct headers available. But every time I pitch that idea I get a ton of push back.

Jun 29 2018, 6:07 PM
smeenai added a comment to D48694: [libc++abi] Limit libc++ header search to specified paths.

LGTM, but I'm a bit confused. You seem to argue that no system places C++ headers on the default search paths, but also that it would be a problem if such a system did.
Why wouldn't the conclusion be true? That is, although C++ includes aren't normally found along the default paths, when they are found, we should still consider them?

That being said, the current behavior of searching certian default include paths first does seem incorrect. So I'm OK with disabling it.

I also agree that we shouldn't necessarily be looking for vector header, how about looking for __config instead, since it's libc++ specific?

Jun 29 2018, 5:34 PM

Jun 28 2018

smeenai accepted D48703: Add natvis files directly to the PDB instead of to the VS solution.

LGTM

Jun 28 2018, 2:19 PM

Jun 27 2018

smeenai added inline comments to D48703: Add natvis files directly to the PDB instead of to the VS solution.
Jun 27 2018, 10:57 PM
smeenai added a comment to D48694: [libc++abi] Limit libc++ header search to specified paths.

This doesn't map to any command line change; it's purely a CMake thing. It just changes where libc++abi's build system looks for libc++ headers. Before this patch, it would look for a file named "vector" in all the standard system include directories (/usr/local/include, /usr/include, anything else in your PATH, etc.) and then look for it in the paths specified by the PATHS argument. After this patch it will only look for that file in the paths specified by the PATHS argument. The actual value of LIBCXXABI_LIBCXX_INCLUDES gets added to the header search path when compiling the libc++abi sources, but this diff isn't changing that part (it just potentially changes the value that gets computed for LIBCXXABI_LIBCXX_INCLUDES, but as I detailed in the description I don't think that should actually change on any system I'm aware of, and if it does cause a change then there's a good chance the old value was incorrect).

Jun 27 2018, 5:58 PM
smeenai abandoned D48675: [libc++abi] Limit libc++ header search to specified paths.

D48694

Jun 27 2018, 5:41 PM
smeenai created D48694: [libc++abi] Limit libc++ header search to specified paths.
Jun 27 2018, 5:41 PM
smeenai updated subscribers of D48675: [libc++abi] Limit libc++ header search to specified paths.

Huh, this went to llvm-commits instead of cfe-commits automatically. I guess the auto mailing list subscribing doesn't work great with the monorepo?

Jun 27 2018, 2:29 PM
smeenai created D48675: [libc++abi] Limit libc++ header search to specified paths.
Jun 27 2018, 2:28 PM
smeenai updated subscribers of D48626: New option -fwindows-filesystem, affecting #include paths..

Adding some people who are interested in Windows cross-compilation.

Jun 27 2018, 1:31 PM
smeenai added a comment to D48620: Fix a single typo in SBSymbolContext.

This is definitely trivial enough to just go through post-commit review :) (As in, just commit it directly without going through Phabricator.)

Jun 27 2018, 1:23 PM

Jun 26 2018

smeenai committed rL335686: [clang] Add test dependency on llvm-as.
[clang] Add test dependency on llvm-as
Jun 26 2018, 4:23 PM
smeenai committed rC335686: [clang] Add test dependency on llvm-as.
[clang] Add test dependency on llvm-as
Jun 26 2018, 4:23 PM

Jun 25 2018

smeenai added a comment to D48459: Respect CMAKE_SYSROOT and CMAKE_CROSSCOMPILING when searching for libxml2..

Okay, upon playing with this further, the following seems to do what I want (for a custom sysroot), and seems to be a pretty common pattern as well:

Jun 25 2018, 5:29 PM
smeenai added a comment to D48459: Respect CMAKE_SYSROOT and CMAKE_CROSSCOMPILING when searching for libxml2..

Ah, the documentation is confusing, but CMAKE_FIND_ROOT_PATH and CMAKE_FIND_ROOT_PATH_MODE_PACKAGE only have an effect when using the config mode of find_package, whereas this invocation is using the module mode. That's a bummer, and definitely makes custom sysroots and cross-compilation harder than it should be. Sorry for the noise.

Jun 25 2018, 5:14 PM
smeenai added a comment to D48459: Respect CMAKE_SYSROOT and CMAKE_CROSSCOMPILING when searching for libxml2..

Actually, I would imagine that if you're cross-compiling or using a custom sysroot, you should probably also specify CMAKE_FIND_ROOT_PATH and set CMAKE_FIND_ROOT_PATH_MODE_PACKAGE to ONLY, to limit all these searches to the desired directories?

Jun 25 2018, 4:45 PM
smeenai added reviewers for D48459: Respect CMAKE_SYSROOT and CMAKE_CROSSCOMPILING when searching for libxml2.: beanz, phosek.

I think it might be more appropriate to pass NO_SYSTEM_ENVIRONMENT_PATH to find_package in case CMAKE_SYSROOT or CMAKE_CROSSCOMPILING are set? That way, we won't search the host system for libxml2 in those cases, but we'll still be able to find it in the custom sysroot or cross-compilation SDK if it's present.

Jun 25 2018, 2:39 PM
smeenai updated subscribers of D42762: Rewrite the VS Integration Scripts.
Jun 25 2018, 12:56 AM

Jun 22 2018

smeenai added inline comments to D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types.
Jun 22 2018, 2:38 PM
smeenai added inline comments to D47988: [CodeGen] Emit MSVC funclet IR for Obj-C exceptions.
Jun 22 2018, 2:37 PM
smeenai added a comment to D48402: [mingw] Fix GCC ABI compatibility for comdat things.
In D48402#1140046, @rnk wrote:

Sections:
....

3 .text$___Z3fooi 00000013  00000000  00000000  0000012c  2**4
               CONTENTS, ALLOC, LOAD, RELOC, READONLY, CODE, LINK_ONCE_DISCARD
Could that explain the difference?

Woops, one too many calls to getNameWithPrefix.

I replaced getMangler().getNameWithPrefix(SecName, Symbol, DL); in appendComdatSymbolForMinGW with SecName.append(Symbol.begin(), Symbol.end());. This removes the extra underscore but still doesn't associate a comdat with the section. Is there anything else that I can try?

Jun 22 2018, 12:21 PM

Jun 20 2018

smeenai added inline comments to D48356: [libcxx] [CMake] Convert paths to the right form in standalone builds on Windows.
Jun 20 2018, 12:47 PM
smeenai accepted D48353: [libunwind] [CMake] Convert paths to the right form in standalone builds on Windows.

LGTM with the same comments as D48356 (add quotes, and is the translation for LLVM_BINARY_DIR necessary?)

Jun 20 2018, 12:33 PM
smeenai accepted D48355: [libcxxabi] [CMake] Convert paths to the right form in standalone builds on Windows.

LGTM with the same comments as D48356 (add quotes, and is the translation for LLVM_BINARY_DIR necessary?)

Jun 20 2018, 12:32 PM
smeenai accepted D48356: [libcxx] [CMake] Convert paths to the right form in standalone builds on Windows.

Looks good with the quotes added.

Jun 20 2018, 12:31 PM

Jun 18 2018

smeenai added a comment to D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types.

Ping.

Jun 18 2018, 11:08 AM
smeenai added a comment to D47988: [CodeGen] Emit MSVC funclet IR for Obj-C exceptions.

Ping.

Jun 18 2018, 11:08 AM

Jun 14 2018

smeenai committed rL334781: [cmake] Change ON/OFF to YES/NO. NFC.
[cmake] Change ON/OFF to YES/NO. NFC
Jun 14 2018, 4:44 PM
smeenai committed rC334780: [cmake] Add linker detection for Apple platforms.
[cmake] Add linker detection for Apple platforms
Jun 14 2018, 4:32 PM
smeenai committed rL334780: [cmake] Add linker detection for Apple platforms.
[cmake] Add linker detection for Apple platforms
Jun 14 2018, 4:32 PM
smeenai closed D48201: [cmake] Add linker detection for Apple platforms.
Jun 14 2018, 4:32 PM
smeenai added inline comments to D48201: [cmake] Add linker detection for Apple platforms.
Jun 14 2018, 4:12 PM
smeenai added a reviewer for D48201: [cmake] Add linker detection for Apple platforms: compnerd.
Jun 14 2018, 4:02 PM
smeenai created D48201: [cmake] Add linker detection for Apple platforms.
Jun 14 2018, 3:33 PM

Jun 13 2018

smeenai committed rL334654: [compiler-rt] Use CMAKE_LINKER instead of hardcoding ld.
[compiler-rt] Use CMAKE_LINKER instead of hardcoding ld
Jun 13 2018, 1:52 PM
smeenai committed rCRT334654: [compiler-rt] Use CMAKE_LINKER instead of hardcoding ld.
[compiler-rt] Use CMAKE_LINKER instead of hardcoding ld
Jun 13 2018, 1:52 PM
smeenai accepted D47994: [Darwin] Do not error on '-lto_library' option.

LGTM

Jun 13 2018, 10:11 AM

Jun 12 2018

smeenai added a comment to D47565: Fix /WholeArchive bug..

This new version is much better; there doesn't actually appear to be any performance difference for my test case at all now (barring noise). Thank you!

Jun 12 2018, 2:41 PM
smeenai committed rLLD334548: [COFF] Fix crash when emitting symbol tables with GC.
[COFF] Fix crash when emitting symbol tables with GC
Jun 12 2018, 2:24 PM
smeenai committed rL334548: [COFF] Fix crash when emitting symbol tables with GC.
[COFF] Fix crash when emitting symbol tables with GC
Jun 12 2018, 2:24 PM
smeenai closed D48092: [COFF] Fix crash when emitting symbol tables with GC.
Jun 12 2018, 2:23 PM
smeenai created D48092: [COFF] Fix crash when emitting symbol tables with GC.
Jun 12 2018, 1:31 PM
smeenai added a comment to D47565: Fix /WholeArchive bug..

This diff results in a 30% link time regression for us (5.3 seconds to 6.8 seconds) when linking ~5,000 input files together. This is when cross-compiling on Linux; the results would probably be even worse on Windows, where I believe stat/the filesystem in general is slower.

Jun 12 2018, 12:25 PM
smeenai added inline comments to D47565: Fix /WholeArchive bug..
Jun 12 2018, 11:48 AM
smeenai added a comment to D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types.

WinObjC does this wrapping, for example.

I see. The idea of creating the type descriptors and mangled names ad-hoc for the catchable-types array is clever, and it's nice that the ABI is defined in a way that makes that work. (Expensive, but hey, it's the exceptions path.)

We ran into some critical issues with this approach on x64. The pointers in the exception record are supposed to be image-relative instead of absolute, so I tried to make them absolute to libobjc2's load address, but I never quite solved it.

A slightly better-documented and cleaner version of the code you linked is here.

Jun 12 2018, 10:48 AM

Jun 11 2018

smeenai added a comment to D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types.

Wrapping an Objective-C exception inside a C++ exception means dynamically constructing a C++ exception object and traversing the class hierarchy of the thrown Obj-C object to populate the catchable types array of the C++ exception. Microsoft's C++ runtime will perform the appropriate catch type comparisons, and this patch makes the compiler emit the right typeinfos into the exception handling tables for all of that to work. https://github.com/Microsoft/libobjc2/blob/f2e4c5ac4b3ac17f413a38bbc7ee1242f9efd0f7/msvc/eh_winobjc.cc#L116 is how WinObjC does this wrapping, for example.

Jun 11 2018, 4:05 PM
smeenai resigned from D47998: [Darwin] Use errorHandler from liblldCommon.

Makes sense to me, but I want @ruiu to look over it.

Jun 11 2018, 12:29 PM
smeenai requested changes to D47994: [Darwin] Do not error on '-lto_library' option.

From what I understand, the way LTO works on LLD COFF and ELF is that the appropriate LLVM libraries are just (statically) linked into LLD itself, so there's no concept of an LTO library. I assume that LTO support in LLD Mach-O will work the same way when it's implemented, so the option should never be needed there either. I think it makes more sense to change the clang driver to not pass the option to LLD, but I'm adding @pcc to confirm that my understanding of LTO is correct.

Jun 11 2018, 12:18 PM

Jun 9 2018

smeenai added a comment to D47988: [CodeGen] Emit MSVC funclet IR for Obj-C exceptions.

@rnk remember how I was asking you about inlining into catchpads on IRC a few days back? It was in relation to this change.

Jun 9 2018, 3:09 PM
smeenai added a dependent revision for D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types: D47988: [CodeGen] Emit MSVC funclet IR for Obj-C exceptions.
Jun 9 2018, 2:55 PM
smeenai added a dependency for D47988: [CodeGen] Emit MSVC funclet IR for Obj-C exceptions: D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types.
Jun 9 2018, 2:55 PM
smeenai created D47988: [CodeGen] Emit MSVC funclet IR for Obj-C exceptions.
Jun 9 2018, 2:54 PM