smeenai (Shoaib Meenai)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 19 2016, 10:21 AM (99 w, 6 d)

Recent Activity

Today

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.

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

Patch is missing context.

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

Yesterday

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.

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

Tue, Jul 17

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

Mon, Jul 16

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.

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

Sun, Jul 15

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

Sat, Jul 14

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

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

Sat, Jul 14, 8:43 PM

Fri, Jul 13

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

LGTM

Fri, Jul 13, 10:47 PM

Thu, Jul 12

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

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

Thu, Jul 12, 4:31 PM · Restricted Project

Wed, Jul 11

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.

Wed, Jul 11, 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?

Wed, Jul 11, 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.

Wed, Jul 11, 10:56 AM

Tue, Jul 10

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

Mon, Jul 9

smeenai added inline comments to D49059: [MinGW] Skip adding default win32 api libraries if -lwindowsapp is specified.
Mon, Jul 9, 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?

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

LGTM, particularly given r314138.

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

LGTM

Mon, Jul 9, 2:06 PM

Fri, Jun 29

smeenai committed rL336034: [libc++abi] Look for __config instead of vector.
[libc++abi] Look for __config instead of vector
Fri, Jun 29, 6:30 PM
smeenai committed rCXXA336034: [libc++abi] Look for __config instead of vector.
[libc++abi] Look for __config instead of vector
Fri, Jun 29, 6:30 PM
smeenai committed rCXXA336032: [libc++abi] Limit libc++ header search to specified paths.
[libc++abi] Limit libc++ header search to specified paths
Fri, Jun 29, 6:13 PM
smeenai committed rL336032: [libc++abi] Limit libc++ header search to specified paths.
[libc++abi] Limit libc++ header search to specified paths
Fri, Jun 29, 6:09 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Fri, Jun 29, 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.

Fri, Jun 29, 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?

Fri, Jun 29, 5:34 PM

Thu, Jun 28

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

LGTM

Thu, Jun 28, 2:19 PM

Wed, Jun 27

smeenai added inline comments to D48703: Add natvis files directly to the PDB instead of to the VS solution.
Wed, Jun 27, 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).

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

D48694

Wed, Jun 27, 5:41 PM
smeenai created D48694: [libc++abi] Limit libc++ header search to specified paths.
Wed, Jun 27, 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?

Wed, Jun 27, 2:29 PM
smeenai created D48675: [libc++abi] Limit libc++ header search to specified paths.
Wed, Jun 27, 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.

Wed, Jun 27, 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.)

Wed, Jun 27, 1:23 PM

Tue, Jun 26

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

Mon, Jun 25

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:

Mon, Jun 25, 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.

Mon, Jun 25, 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?

Mon, Jun 25, 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.

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

Fri, Jun 22

smeenai added inline comments to D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types.
Fri, Jun 22, 2:38 PM
smeenai added inline comments to D47988: [CodeGen] Emit MSVC funclet IR for Obj-C exceptions.
Fri, Jun 22, 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?

Fri, Jun 22, 12:21 PM

Wed, Jun 20

smeenai added inline comments to D48356: [libcxx] [CMake] Convert paths to the right form in standalone builds on Windows.
Wed, Jun 20, 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?)

Wed, Jun 20, 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?)

Wed, Jun 20, 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.

Wed, Jun 20, 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

Jun 7 2018

smeenai committed rL334253: [CodeGen] Always use MSVC personality for windows-msvc targets.
[CodeGen] Always use MSVC personality for windows-msvc targets
Jun 7 2018, 5:45 PM
smeenai committed rC334253: [CodeGen] Always use MSVC personality for windows-msvc targets.
[CodeGen] Always use MSVC personality for windows-msvc targets
Jun 7 2018, 5:45 PM
smeenai closed D47862: [CodeGen] Always use MSVC personality for windows-msvc targets.
Jun 7 2018, 5:45 PM
Herald added a reviewer for D42762: Rewrite the VS Integration Scripts: javed.absar.

Any updates here?

Jun 7 2018, 5:44 PM
smeenai committed rL334251: Reapply "[Parse] Use CapturedStmt for @finally on MSVC".
Reapply "[Parse] Use CapturedStmt for @finally on MSVC"
Jun 7 2018, 5:34 PM
smeenai committed rC334251: Reapply "[Parse] Use CapturedStmt for @finally on MSVC".
Reapply "[Parse] Use CapturedStmt for @finally on MSVC"
Jun 7 2018, 5:34 PM
smeenai added inline comments to D47862: [CodeGen] Always use MSVC personality for windows-msvc targets.
Jun 7 2018, 5:17 PM
smeenai committed rC334243: [Frontend] Disallow non-MSVC exception models for windows-msvc targets.
[Frontend] Disallow non-MSVC exception models for windows-msvc targets
Jun 7 2018, 3:59 PM
smeenai committed rL334243: [Frontend] Disallow non-MSVC exception models for windows-msvc targets.
[Frontend] Disallow non-MSVC exception models for windows-msvc targets
Jun 7 2018, 3:59 PM
smeenai closed D47853: [Frontend] Disallow non-MSVC exception models for windows-msvc targets.
Jun 7 2018, 3:59 PM
smeenai added a comment to D47912: [CMake] Consider LLVM_APPEND_VC_REV when generating SVNVersion.inc.

I believe LLVM_APPEND_VC_REV controls whether the revision is appended to LLVM version string (e.g. the output of llvm-config --version), whereas the SVNRevision.inc file (which is what's causing the relink after amending) is used for e.g. the clang --version output, so I'm not sure this is the right thing to do. I would also love for an option to disable the SVNRevision stuff entirely though.

Jun 7 2018, 3:33 PM
smeenai committed rL334240: Revert "[Parse] Use CapturedStmt for @finally on MSVC".
Revert "[Parse] Use CapturedStmt for @finally on MSVC"
Jun 7 2018, 3:28 PM
smeenai committed rC334240: Revert "[Parse] Use CapturedStmt for @finally on MSVC".
Revert "[Parse] Use CapturedStmt for @finally on MSVC"
Jun 7 2018, 3:28 PM
smeenai added a comment to D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types.

Ping @rjmccall

Jun 7 2018, 1:28 PM
smeenai updated the diff for D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types.

Address objc_exception attribute case

Jun 7 2018, 1:27 PM
smeenai committed rC334224: [Parse] Use CapturedStmt for @finally on MSVC.
[Parse] Use CapturedStmt for @finally on MSVC
Jun 7 2018, 1:12 PM
smeenai committed rL334224: [Parse] Use CapturedStmt for @finally on MSVC.
[Parse] Use CapturedStmt for @finally on MSVC
Jun 7 2018, 1:12 PM
smeenai closed D47564: [Parse] Use CapturedStmt for @finally on MSVC.
Jun 7 2018, 1:12 PM
smeenai updated the diff for D47853: [Frontend] Disallow non-MSVC exception models for windows-msvc targets.

Add back missing MinGW coverage

Jun 7 2018, 12:59 PM
smeenai added inline comments to D47853: [Frontend] Disallow non-MSVC exception models for windows-msvc targets.
Jun 7 2018, 12:57 PM

Jun 6 2018

smeenai added a dependency for D47862: [CodeGen] Always use MSVC personality for windows-msvc targets: D47853: [Frontend] Disallow non-MSVC exception models for windows-msvc targets.
Jun 6 2018, 7:12 PM
smeenai added a dependent revision for D47853: [Frontend] Disallow non-MSVC exception models for windows-msvc targets: D47862: [CodeGen] Always use MSVC personality for windows-msvc targets.
Jun 6 2018, 7:12 PM