Page MenuHomePhabricator

zahiraam (Zahira Ammarguellat)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 14 2017, 11:28 AM (138 w, 4 d)

Recent Activity

Mon, Dec 9

zahiraam committed rG32c802e0f539: Fix build bot fails due to the patch here: https://reviews.llvm.org/D70691… (authored by zahiraam).
Fix build bot fails due to the patch here: https://reviews.llvm.org/D70691…
Mon, Dec 9, 6:32 AM
zahiraam added a comment to D70691: Optimization record for bytecode input missing- PR44000.

I looked at the test failure, the REQUIRES line is necessary because the test calls emit-obj. I've asked Zahira to commit it to fix the bots.

Mon, Dec 9, 6:31 AM · Restricted Project
zahiraam added a comment to D70691: Optimization record for bytecode input missing- PR44000.

Got other build fails.
Doesn't the LIT test require a:
// REQUIRES: x86-registered-target
command?

Mon, Dec 9, 12:33 AM · Restricted Project

Sun, Dec 8

zahiraam committed rG27f5d35137cb: Fix for build bot failure. For more details see: https://reviews.llvm. (authored by zahiraam).
Fix for build bot failure. For more details see: https://reviews.llvm.
Sun, Dec 8, 9:58 PM
zahiraam added a comment to D70691: Optimization record for bytecode input missing- PR44000.

LGTM. Could you commit this soon to get the bots green again?

Sun, Dec 8, 9:58 PM · Restricted Project

Fri, Dec 6

zahiraam added a comment to D70691: Optimization record for bytecode input missing- PR44000.

This might be easier to read the edit that I have made:
ksh-3.2$ git diff
diff --git a/clang/test/CodeGen/opt-record-1.c b/clang/test/CodeGen/opt-record-1.c
index 3f37e32..00a753d 100644

  • a/clang/test/CodeGen/opt-record-1.c

+++ b/clang/test/CodeGen/opt-record-1.c
@@ -1,12 +1,12 @@
- RUN: %clang_cc1 %s -opt-record-file=t1.opt -fopenmp -emit-llvm-bc -o %t.bc
-
RUN: %clang_cc1 -x ir %t.bc -opt-record-file %t.opt -fopenmp -emit-obj
+ RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -target-cpu x86-64 %s -O3 -opt-record-file=t1.opt -fopenmp -emit-llvm-bc -o %t.bc
+
RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -target-cpu x86-64 -O3 -x ir %t.bc -opt-record-file %t.opt -fopenmp -emit-obj
// RUN: cat %t.opt | FileCheck -check-prefix=CHECK %s

Fri, Dec 6, 2:34 PM · Restricted Project
zahiraam added a comment to D70691: Optimization record for bytecode input missing- PR44000.

If you agree with the edit I will commit the change. Thanks.

Fri, Dec 6, 2:25 PM · Restricted Project
zahiraam updated the diff for D70691: Optimization record for bytecode input missing- PR44000.
Fri, Dec 6, 2:25 PM · Restricted Project
zahiraam added a comment to D70691: Optimization record for bytecode input missing- PR44000.

Please see the edit in the list test. Fixed the build issue that occurred in the buildbot:
http://lab.llvm.org:8011/builders/clang-x86_64-debian-new-pass-manager-fast/builds/232/steps/test-check-all/logs/stdio

Fri, Dec 6, 2:25 PM · Restricted Project
zahiraam committed rGa3b2552575d3: Fix for PR44000. Optimization record for bytecode input missing. Review is here… (authored by zahiraam).
Fix for PR44000. Optimization record for bytecode input missing. Review is here…
Fri, Dec 6, 4:57 AM

Wed, Dec 4

zahiraam updated the diff for D70691: Optimization record for bytecode input missing- PR44000.
Wed, Dec 4, 9:51 AM · Restricted Project
zahiraam added a comment to D70691: Optimization record for bytecode input missing- PR44000.

Fixed the indentation. Thanks.

Wed, Dec 4, 9:51 AM · Restricted Project
zahiraam updated the diff for D70691: Optimization record for bytecode input missing- PR44000.
Wed, Dec 4, 8:45 AM · Restricted Project
zahiraam added a comment to D70691: Optimization record for bytecode input missing- PR44000.

Changed it. Thanks.

Wed, Dec 4, 8:45 AM · Restricted Project
zahiraam added a comment to D70691: Optimization record for bytecode input missing- PR44000.

Thanks for the review. Please let me know if I have addressed everything.

Wed, Dec 4, 8:08 AM · Restricted Project
zahiraam updated the diff for D70691: Optimization record for bytecode input missing- PR44000.
Wed, Dec 4, 8:08 AM · Restricted Project

Fri, Nov 29

zahiraam added a reviewer for D70691: Optimization record for bytecode input missing- PR44000: erichkeane.
Fri, Nov 29, 6:03 AM · Restricted Project
zahiraam added a comment to D70691: Optimization record for bytecode input missing- PR44000.

After an (offline) review from @erichkeane (added now as a reviewer) , stripped the unnecessary code. Thanks.

Fri, Nov 29, 6:03 AM · Restricted Project
zahiraam updated the diff for D70691: Optimization record for bytecode input missing- PR44000.
Fri, Nov 29, 6:03 AM · Restricted Project

Tue, Nov 26

zahiraam added inline comments to D70691: Optimization record for bytecode input missing- PR44000.
Tue, Nov 26, 7:30 AM · Restricted Project
zahiraam updated the diff for D70691: Optimization record for bytecode input missing- PR44000.
Tue, Nov 26, 7:30 AM · Restricted Project

Mon, Nov 25

zahiraam added a reviewer for D70691: Optimization record for bytecode input missing- PR44000: hfinkel.
Mon, Nov 25, 11:50 AM · Restricted Project
zahiraam created D70691: Optimization record for bytecode input missing- PR44000.
Mon, Nov 25, 11:50 AM · Restricted Project

Oct 14 2019

zahiraam added inline comments to D68521: [PATCH 36/38] [noalias] Clang CodeGen for restrict-qualified pointers.
Oct 14 2019, 2:13 PM · Restricted Project

Aug 8 2019

zahiraam added a comment to D65774: Removing redundant-move warnings generated by gcc9x..

Agree with all these comments -- I'm no expert on when and when not to std::move, but this is undoing changes I've made to fix buildbots on different compilers.

Aug 8 2019, 5:04 AM · Restricted Project

Aug 5 2019

zahiraam created D65774: Removing redundant-move warnings generated by gcc9x..
Aug 5 2019, 1:39 PM · Restricted Project

Jul 3 2019

zahiraam added inline comments to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Jul 3 2019, 1:39 PM
zahiraam updated the diff for D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

Thanks for the review.
I think and hope that I have responded to every issue you raised. Let me know if there are still pending issues.
Happy 4th!

Jul 3 2019, 1:39 PM

May 30 2019

zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

A review please :-) Thanks.

May 30 2019, 7:07 AM

May 21 2019

zahiraam updated the diff for D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
May 21 2019, 6:24 AM
zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

And this patch actually fixes the bug. Thanks.

May 21 2019, 6:24 AM

May 20 2019

zahiraam updated the diff for D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
May 20 2019, 5:56 AM
zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

@rsmith I think I have made all the changes you have pointed out to. But please note that this new patch only impements an explicit AST representation of uuid in template arguments. It does not fix the bug for which this review was opened for.
I will take care of the bug in time. But before doing that I want to make sure that the changes required to give an explicit AST for uuid is correct.

May 20 2019, 5:55 AM

Apr 2 2019

zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

There are still a few things I haven't addressed yet. Mostly because not sure there is another solution like getting rid of the map from StringRef to expr. The other issue is not adding new kind to ParsedAttr. There may be another way of doing it, but didn't look at it yet.
Meanwhile can you please let me know if you are happy with the current status of the implementation.

Apr 2 2019, 5:22 AM
zahiraam updated the diff for D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Apr 2 2019, 5:19 AM

Mar 19 2019

zahiraam updated the diff for D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Mar 19 2019, 12:56 PM
zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

It would be nice to have a review for this year old (updated) patch. Thanks.

Mar 19 2019, 12:56 PM

Mar 18 2019

zahiraam added a comment to D45978: dllexport const variables must have external linkage..

LGTM!

Mar 18 2019, 6:49 AM

Mar 11 2019

zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Mar 11 2019, 10:55 AM
zahiraam added inline comments to D45978: dllexport const variables must have external linkage..
Mar 11 2019, 10:53 AM

Mar 8 2019

zahiraam added inline comments to D45978: dllexport const variables must have external linkage..
Mar 8 2019, 4:36 AM
zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Mar 8 2019, 4:36 AM

Mar 7 2019

zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Mar 7 2019, 12:33 AM

Feb 25 2019

zahiraam added a comment to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
In D41950#1404899, @rnk wrote:

lgtm

Feb 25 2019, 9:41 AM · Restricted Project

Feb 23 2019

zahiraam added a comment to D45978: dllexport const variables must have external linkage..

Let's see if I have included every thing mentioned. Thanks.

Feb 23 2019, 6:40 AM
zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Feb 23 2019, 6:40 AM

Feb 20 2019

zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Feb 20 2019, 7:14 AM · Restricted Project
zahiraam added inline comments to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Feb 20 2019, 7:14 AM · Restricted Project

Feb 14 2019

zahiraam added inline comments to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Feb 14 2019, 6:54 AM · Restricted Project
zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Feb 14 2019, 6:54 AM · Restricted Project
zahiraam added inline comments to D45978: dllexport const variables must have external linkage..
Feb 14 2019, 2:24 AM

Feb 13 2019

zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Feb 13 2019, 6:36 AM
zahiraam added a comment to D45978: dllexport const variables must have external linkage..
Feb 13 2019, 6:36 AM

Feb 12 2019

zahiraam updated subscribers of D45978: dllexport const variables must have external linkage..
Feb 12 2019, 7:10 AM
zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Feb 12 2019, 7:10 AM

Feb 11 2019

zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Feb 11 2019, 9:14 AM
zahiraam added a comment to D45978: dllexport const variables must have external linkage..

It looks like the patch got mucked up somehow, I only see three testing files in the patch now?

Feb 11 2019, 9:14 AM

Feb 8 2019

zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Feb 8 2019, 12:37 AM
zahiraam added a comment to D45978: dllexport const variables must have external linkage..

Can you add tests for C mode as well, as it seems the behavior differs there.

Feb 8 2019, 12:37 AM

Feb 6 2019

zahiraam added a comment to D45978: dllexport const variables must have external linkage..

That seems like a reasonable place to try, to me.

Feb 6 2019, 6:33 AM
zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Feb 6 2019, 6:33 AM

Feb 3 2019

zahiraam added a comment to D45978: dllexport const variables must have external linkage..
In D45978#1379901, @rnk wrote:

I'm still not sure this is the best place to make this change, but the functionality is important. There are still unaddressed comments (no need to check MSVCCompatibility, formatting), and I think once those are fixed we can land this.

Feb 3 2019, 3:17 AM

Jan 30 2019

zahiraam added a comment to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..

Feedback please. Thanks.

Jan 30 2019, 5:50 AM · Restricted Project

Jan 26 2019

zahiraam added a comment to D45978: dllexport const variables must have external linkage..

Please advise.
Thanks.

Jan 26 2019, 2:07 AM

Nov 24 2018

zahiraam added a comment to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..

Feedback on this please. Thanks.

Nov 24 2018, 3:39 AM · Restricted Project

May 14 2018

zahiraam updated the diff for D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
May 14 2018, 10:50 AM

May 1 2018

zahiraam added a comment to D46226: Oenmp task with by reference array.

Is this better? Thanks.

May 1 2018, 8:09 AM
zahiraam updated the diff for D46226: Oenmp task with by reference array.
May 1 2018, 8:09 AM

Apr 28 2018

zahiraam added a comment to D46226: Oenmp task with by reference array.
Apr 28 2018, 9:08 AM
zahiraam created D46226: Oenmp task with by reference array.
Apr 28 2018, 9:08 AM

Apr 25 2018

zahiraam added a comment to D45978: dllexport const variables must have external linkage..
Apr 25 2018, 11:05 AM
zahiraam added a comment to D45978: dllexport const variables must have external linkage..
In D45978#1077051, @rnk wrote:

This shouldn't be conditional under -fms-compatibility, it's part of dllexport, which is already under -fms-extensions / -fdeclspec-extensions.

I don't think this is the right place to do this. Can it be done as part of processing the dllexport attribute or as part of the main storage class calculation?

Apr 25 2018, 11:05 AM
zahiraam updated the diff for D45978: dllexport const variables must have external linkage..
Apr 25 2018, 11:04 AM

Apr 23 2018

zahiraam created D45978: dllexport const variables must have external linkage..
Apr 23 2018, 11:48 AM

Apr 20 2018

zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

Richard,
Please let me know if I have answered to all the issues you raised. Thanks.

Apr 20 2018, 8:33 AM
zahiraam updated the diff for D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Apr 20 2018, 8:33 AM
zahiraam added inline comments to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Apr 20 2018, 8:30 AM

Apr 15 2018

zahiraam added inline comments to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Apr 15 2018, 9:06 AM

Apr 14 2018

zahiraam updated the diff for D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).
Apr 14 2018, 8:25 AM
zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

We should really, really avoid making this sort of change without first trying to desugar uuidof into a reference to a variable. That would solve a ton of problems, problems like this one.

Apr 14 2018, 8:20 AM

Apr 13 2018

zahiraam added a comment to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Apr 13 2018, 8:14 AM · Restricted Project
zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Apr 13 2018, 8:14 AM · Restricted Project

Apr 11 2018

zahiraam added a comment to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Apr 11 2018, 9:45 AM · Restricted Project
zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Apr 11 2018, 9:45 AM · Restricted Project

Apr 9 2018

zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Apr 9 2018, 4:12 AM · Restricted Project

Mar 20 2018

zahiraam accepted rL327754: [MS] Fix bug in r327732 with devirtualized complete destructor calls.
Mar 20 2018, 11:50 AM

Mar 19 2018

zahiraam raised a concern with rL327754: [MS] Fix bug in r327732 with devirtualized complete destructor calls.

This is the LLVM I get. The above test case is failing:

Mar 19 2018, 3:19 PM
zahiraam added a comment to rL327754: [MS] Fix bug in r327732 with devirtualized complete destructor calls.

I think this breaks CodeGenCXX/dllimport-base.cpp and devirtualize-ms-dtor.cpp?

Mar 19 2018, 6:10 AM

Mar 16 2018

zahiraam accepted D44505: [MS] Always use base dtors in place of complete/vbase dtors when possible.
Mar 16 2018, 12:08 PM
zahiraam added a comment to D44505: [MS] Always use base dtors in place of complete/vbase dtors when possible.

LGTM 2. It fixes PR32990.

Mar 16 2018, 12:08 PM

Mar 9 2018

zahiraam added a comment to D39063: Fix for PR32990..
Mar 9 2018, 1:25 PM
zahiraam updated the diff for D39063: Fix for PR32990..
Mar 9 2018, 1:25 PM

Mar 8 2018

zahiraam added a comment to D38803: Suppress generation of a deleting destructor for a dllimport record..

Attached the test case.

Mar 8 2018, 11:59 AM
zahiraam updated the diff for D38803: Suppress generation of a deleting destructor for a dllimport record..

Test case to add.

Mar 8 2018, 11:59 AM
zahiraam added a comment to D38803: Suppress generation of a deleting destructor for a dllimport record..
In D38803#1030905, @rnk wrote:

Can you check the base revision you used to upload the patch? I think there should just be new test case code now.

Yes. Can you please commit the test case?

I believe he's saying you should rebase, since an additional testcase has already been added.

Oops! I thought I had done it already. Sorry. On it.

Mar 8 2018, 7:11 AM
zahiraam added a comment to D38803: Suppress generation of a deleting destructor for a dllimport record..
In D38803#1030905, @rnk wrote:

Can you check the base revision you used to upload the patch? I think there should just be new test case code now.

Mar 8 2018, 6:24 AM

Mar 2 2018

zahiraam added inline comments to D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..
Mar 2 2018, 6:27 AM · Restricted Project
zahiraam updated the diff for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword..

Hopefully I have responded to the issues you have raised.
I have added the test case you have submitted, to make sure that an expression is generated.
Some of the error messages in the lit test have to be updated (they do match MSVC error messages).

Mar 2 2018, 6:26 AM · Restricted Project

Feb 28 2018

zahiraam added reviewers for D41950: Fix for Bug 8446. Template instantiation without a 'typename' keyword.: hans, erichkeane.
Feb 28 2018, 6:09 AM · Restricted Project

Feb 26 2018

zahiraam added a comment to D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid).

Here's my thinking: the __uuidof expression literally declares a variable called _GUID_ddb47a6a_0f23_11d5_9109_00e0296b75d3 of type __s_GUID which is why it behaves the way it does: https://godbolt.org/g/74FY7U

This is an implementation detail leaking, though. no? Note that that is a reserved name.

I don't think it is reasonable to invent new semantics which are different from the MSVC ones because we find the MSVC ones inelegant.

I mostly agree, but my point is that this is *not* the MSVC semantics, it's merely an implementation detail that non-conforming code happens to be able to observe. Suppose that type_info objects were similarly accessible in MSVC by guessing their mangled names. Would you be arguing that we should inject variables for those too? (And note that it is *nearly* true that type_info objects work that way: https://godbolt.org/g/zByFFg -- but the parser gets confused somehow when you reference them.) The only difference I can see between these cases is that the reserved name used for the GUID case happens to not contain any ?s and @s, so happens to be utterable as an identifier.

We should not attempt to be compatible with the cases where MSVC's implementation details happen to leak into user-visible semantics.

What is the relative upside to a new kind of Decl? Better AST fidelity?

Yes, exactly. The same reason we don't desguar other things any more than we have to.

Feb 26 2018, 1:03 PM