# Projects

User does not belong to any projects.

# User Details

User Since
Dec 1 2019, 10:41 PM (199 w, 4 h)

# Sep 30 2020

Jac1494 updated the diff for D88551: [InstCombine] Test cases added for pattern "(~A & B) ^ A -> (A | B)".

Updated full patch to solve "content not available" error.

Sep 30 2020, 4:09 AM · Restricted Project
Jac1494 updated the diff for D86395: [InstCombine] transform pattern "(~A & B) ^ A -> (A | B)" added.

Separate review created for test cases. (https://reviews.llvm.org/D88551)

Sep 30 2020, 3:31 AM · Restricted Project
Sep 30 2020, 3:28 AM · Restricted Project

# Sep 15 2020

test53 and test55 has been removed.

We are going in circles. I'll give this 1 more try, and if it still doesn't make sense, then maybe another reviewer can better communicate what we expect for the tests:

1. There are 4 commutative patterns variations in the code.
2. Each one of those patterns should have a test (we should have at least 4 tests).
3. Each test corresponds to exactly one of the commutative patterns.
4. Each test should be in canonical form without this patch (the baseline CHECK lines should correspond exactly to the IR as written).
5. The 8 step check I provided earlier may be used to confirm that the tests provide the expected coverage for the code.

@spatel, apologies for the confusion created due to multiple revisions.

Let's consider the initial pattern and associated commutative version of this.

```initial pattern:-
(~A & B) ^ A -> (A | B)

And 8 possible commutative version of this.
1) (~A & B) ^ A -> (A | B)
2) (~B & A) ^ B -> (A | B)
3) (B & ~A) ^ A -> (A | B)  --> identical to 1)
4) A ^ (~A & B) -> (A | B)  --> identical to 1)
5) A ^ (B & ~A) -> (A | B)  --> identical to 1)
6) (A & ~B) ^ B -> (A | B)  --> identical to 2)
7) B ^ (~B & A) -> (A | B)  --> identical to 2)
8) B ^ (A & ~B) -> (A | B)  --> identical to 2)```

As you may notice that all these patterns (IR) are reducing to 2 unique patterns (pattern 1 and pattern 2).
So pattern 1 and 2 are sufficient for the coverage of all commutative variations of the primary pattern.

How so?
If you only have a test for pattern 1 and pattern 2, how do you know that the commutative variants are handled?

Does this matches with you understandings ??

Sep 15 2020, 9:26 AM · Restricted Project

test53 and test55 has been removed.

We are going in circles. I'll give this 1 more try, and if it still doesn't make sense, then maybe another reviewer can better communicate what we expect for the tests:

1. There are 4 commutative patterns variations in the code.
2. Each one of those patterns should have a test (we should have at least 4 tests).
3. Each test corresponds to exactly one of the commutative patterns.
4. Each test should be in canonical form without this patch (the baseline CHECK lines should correspond exactly to the IR as written).
5. The 8 step check I provided earlier may be used to confirm that the tests provide the expected coverage for the code.
Sep 15 2020, 9:10 AM · Restricted Project

# Sep 13 2020

Gentle Ping !!

Sep 13 2020, 11:28 AM · Restricted Project, Restricted Project

# Sep 11 2020

Jac1494 updated the diff for D86395: [InstCombine] transform pattern "(~A & B) ^ A -> (A | B)" added.

test53 and test55 has been removed.

Sep 11 2020, 11:33 AM · Restricted Project

Try this experiment to see if your tests provide the coverage that we expect:

1. Replace the "m_c_XXX" matchers with the non-commute equivalent.
2. Exactly 1 test should be modified.
3. Add back only 1 of the commutative matchers.
4. Exactly 2 tests should be modified.
5. Invert the commutativity of the matchers (so the opposite matcher is now commutative).
6. Exactly 2 tests should be modified.
7. Make both matchers commutative.
8. All 4 tests should be modified.

Did you try this with the tests as shown in this draft of the patch?

Yes, @spatel I have tried this cases with tests of this patch.

Then I think something is wrong with your build environment. If I remove the commutative matchers, I get 0 test diffs.

@spatel it is also same for me. If I remove both the commutative matchers than I also get 0 test diffs.(change x_c_Xor with x_Xor and x_c_And with x_And)

Sep 11 2020, 10:43 AM · Restricted Project

Try this experiment to see if your tests provide the coverage that we expect:

1. Replace the "m_c_XXX" matchers with the non-commute equivalent.
2. Exactly 1 test should be modified.
3. Add back only 1 of the commutative matchers.
4. Exactly 2 tests should be modified.
5. Invert the commutativity of the matchers (so the opposite matcher is now commutative).
6. Exactly 2 tests should be modified.
7. Make both matchers commutative.
8. All 4 tests should be modified.

Did you try this with the tests as shown in this draft of the patch?

Sep 11 2020, 9:15 AM · Restricted Project
Jac1494 updated the diff for D86395: [InstCombine] transform pattern "(~A & B) ^ A -> (A | B)" added.

Sep 11 2020, 8:52 AM · Restricted Project

# Sep 10 2020

Jac1494 updated the diff for D86395: [InstCombine] transform pattern "(~A & B) ^ A -> (A | B)" added.

Test cases are tested and updated.

Sep 10 2020, 11:57 AM · Restricted Project

# Sep 7 2020

Jac1494 updated the diff for D86395: [InstCombine] transform pattern "(~A & B) ^ A -> (A | B)" added.

@spatel test cases are updated as per your review comments. Thanks for this.
And these test cases were failing without this patch. So it means that patch is doing correct transformations.

Sep 7 2020, 9:36 AM · Restricted Project

# Sep 3 2020

Hi All,

Sep 3 2020, 9:39 AM · Restricted Project, debug-info

# Sep 2 2020

If my earlier comment about the tests did not make sense, please grep for "thwart" in this test directory to see examples.

Sep 2 2020, 10:12 AM · Restricted Project

# Aug 29 2020

Jac1494 updated the diff for D86363: InstCombine transform pattern "(A ^ B) | ~(A | B) -> ~(A & B)" added.

Removed unnecessary code.

Aug 29 2020, 10:02 AM · Restricted Project, Restricted Project
Jac1494 updated the diff for D86395: [InstCombine] transform pattern "(~A & B) ^ A -> (A | B)" added.

Removed unnecessary code.

Aug 29 2020, 9:22 AM · Restricted Project
Jac1494 updated the diff for D86395: [InstCombine] transform pattern "(~A & B) ^ A -> (A | B)" added.

Aug 29 2020, 8:55 AM · Restricted Project

You are proposing to transform 4 commuted variations of a pattern, but your tests do not provide coverage for those variants. My test suggestion would hopefully provide that coverage.
This might be clearer if you create the tests before this code patch and step through the transforms in a debugger. Look for InstCombinerImpl::SimplifyAssociativeOrCommutative() and getComplexity().

Aug 29 2020, 7:40 AM · Restricted Project
Jac1494 updated the diff for D86363: InstCombine transform pattern "(A ^ B) | ~(A | B) -> ~(A & B)" added.

New pattern "~(A | B) | (A ^ B) -> ~(A & B)" and Test cases are added.

Aug 29 2020, 6:37 AM · Restricted Project, Restricted Project

# Aug 27 2020

The tests should be pre-committed with current CHECK lines (ie, without this patch), so we just see the diffs - and can confirm that the tests actually represent the patterns shown in the test comments.

Aug 27 2020, 7:28 AM · Restricted Project
Jac1494 updated the diff for D86395: [InstCombine] transform pattern "(~A & B) ^ A -> (A | B)" added.

Added new pattern "A ^ (~A & B) --> (A | B)" .

Aug 27 2020, 7:21 AM · Restricted Project

# Aug 25 2020

Jac1494 added a comment to D83468: [Debuginfo] Fix for PR46653.

void Instruction::updateLocationAfterHoist() {

```const DebugLoc &DL = getDebugLoc();
if (!DL)
return;```

Code Snippet from link (https://reviews.llvm.org/D85670) is not setting the line number at all.
And as seen/observed ,we can also prevent jumpy behavior by skipping the line number(By not setting to line number zero).

I'm sharing the code snippet to avoid unnecessary revision hindering the actual discussion.Is this Okey ? Or should I revise ?

That snippet is just returning if there was no debug info at all, there's a ! in the if condition.

Sorry for misunderstanding , I am also trying to address no debug location.
snippet shared above suggest the removal of this snippet from IRTransaltor.cpp

```// We only emit constants into the entry block from here. To prevent jumpy
// debug behaviour set the line to 0.
if (const DebugLoc &DL = Inst.getDebugLoc())
EntryBuilder->setDebugLoc(
DebugLoc::get(0, 0, DL.getScope(), DL.getInlinedAt()));
else
EntryBuilder->setDebugLoc(DebugLoc());```

Have you ruled this out as a gdb bug/possible improvement?

Yes , Because Arm64 with selectionDAG works fine. and we can also prevent jumpy behavior by skipping the line number(By not setting to line number zero), this way we can correct Global-isel behavior too.

Aug 25 2020, 11:54 AM · Restricted Project, debug-info
Jac1494 added a comment to D83468: [Debuginfo] Fix for PR46653.

But it's possible that the right solution is to drop the location, rather than use line zero here. Perhaps this code in GlobalISel should be using updateLocationAfterHoist ( https://reviews.llvm.org/D85670 ) @vsk @aprantl

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:2183

```// We only emit constants into the entry block from here. To prevent jumpy
// debug behaviour set the line to 0.
if (const DebugLoc &DL = Inst.getDebugLoc())
EntryBuilder->setDebugLoc(
DebugLoc::get(0, 0, DL.getScope(), DL.getInlinedAt()));
else
EntryBuilder->setDebugLoc(DebugLoc());```
Aug 25 2020, 10:25 AM · Restricted Project, debug-info

# Aug 24 2020

Jac1494 updated the diff for D86395: [InstCombine] transform pattern "(~A & B) ^ A -> (A | B)" added.

Aug 24 2020, 12:22 PM · Restricted Project

Are you ok with this..?

Aug 24 2020, 10:37 AM · Restricted Project, Restricted Project

# Aug 22 2020

Jac1494 updated the diff for D86363: InstCombine transform pattern "(A ^ B) | ~(A | B) -> ~(A & B)" added.

Aug 22 2020, 7:44 AM · Restricted Project, Restricted Project
Jac1494 added a reviewer for D86395: [InstCombine] transform pattern "(~A & B) ^ A -> (A | B)" added: xbolva00.
Aug 22 2020, 7:07 AM · Restricted Project
Jac1494 updated the diff for D86395: [InstCombine] transform pattern "(~A & B) ^ A -> (A | B)" added.

Aug 22 2020, 7:05 AM · Restricted Project
Jac1494 requested review of D86395: [InstCombine] transform pattern "(~A & B) ^ A -> (A | B)" added.
Aug 22 2020, 4:45 AM · Restricted Project
Jac1494 added inline comments to D86363: InstCombine transform pattern "(A ^ B) | ~(A | B) -> ~(A & B)" added.
Aug 22 2020, 3:35 AM · Restricted Project, Restricted Project
Jac1494 updated the diff for D86363: InstCombine transform pattern "(A ^ B) | ~(A | B) -> ~(A & B)" added.

Aug 22 2020, 3:32 AM · Restricted Project, Restricted Project

# Aug 21 2020

Aug 21 2020, 12:35 PM · Restricted Project, Restricted Project

# Aug 19 2020

Jac1494 added a comment to D83468: [Debuginfo] Fix for PR46653.

Also possible it's not a bug at all & GlobalISel just ends up doing some optimization that does end up creating a necessary/correct line 0 at the start of the function where SelectionDAG/FastISel does not.

Aug 19 2020, 12:19 PM · Restricted Project, debug-info
Jac1494 added a comment to D83468: [Debuginfo] Fix for PR46653.

I would consider this to be more of a bug in GDB. Line 0 has a well-defined meaning in DWARF and stands for "no source location corresponds to this code". LLDB, for example, interprets line 0 as an indicator to skip over instructions and automatically steps over them when single-stepping through a program, since line 0 is not a meaningful place to stop. It might be worth talking to the GDB developers and ask them what they think about adding a similar feature to their handling of line 0. DWARF doesn't prescribe that that's how debuggers should behave, but I think it's a reasonable interpretation.

Aug 19 2020, 9:43 AM · Restricted Project, debug-info

# Aug 18 2020

Jac1494 added a comment to D83468: [Debuginfo] Fix for PR46653.

The IRTranslator and Legalizer do not set the line number, they're supposed to just forward the line number it already has.

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:2183

Aug 18 2020, 11:40 AM · Restricted Project, debug-info
Jac1494 added a comment to D83468: [Debuginfo] Fix for PR46653.

If the problem that you want to solve is that the prologue ends at line 0, then massaging earlier passes until the right result happens to come up in one testcase is not going to be very effective. Yes, it may fix the testcase, but that won't last long, and the patch is introducing questionable behavior that will end up causing other problems.

Aug 18 2020, 11:13 AM · Restricted Project, debug-info

# Aug 17 2020

Aug 17 2020, 12:23 PM · Restricted Project, debug-info
Jac1494 added a reviewer for D83468: [Debuginfo] Fix for PR46653: arsenm.
Aug 17 2020, 11:08 AM · Restricted Project, debug-info

# Aug 16 2020

Jac1494 added a comment to D83468: [Debuginfo] Fix for PR46653.

PING??

Aug 16 2020, 11:50 PM · Restricted Project, debug-info

# Aug 14 2020

Jac1494 added a comment to D83468: [Debuginfo] Fix for PR46653.

https://github.com/llvm/llvm-project/commit/32823091c36cfa2b27b717246f15d4f12591e6f4

Aug 14 2020, 3:16 AM · Restricted Project, debug-info

# Aug 13 2020

Jac1494 added a comment to D83468: [Debuginfo] Fix for PR46653.

I think I haven't quit understood what your goal is. Are you trying to avoid having the prologue end at line 0?

Yes, Also other line 0 too which is making incorrect debugging experience.

Aug 13 2020, 10:14 AM · Restricted Project, debug-info

# Aug 12 2020

Jac1494 added a comment to D83468: [Debuginfo] Fix for PR46653.

PING?

Aug 12 2020, 12:54 AM · Restricted Project, debug-info

# Aug 11 2020

Jac1494 updated the diff for D83468: [Debuginfo] Fix for PR46653.

Issue is with Global-isel , So trying to handle issue in Legalizer. Also tested debugging behavior and it is fine.

Aug 11 2020, 1:31 PM · Restricted Project, debug-info

# Aug 10 2020

Jac1494 added a comment to D83468: [Debuginfo] Fix for PR46653.

Thanks for posting the line table. It appears to confirm my suspicion that this patch merely hides the issue, but it isn't correct. To be sure it may be possible to preserve the debug location in this function, but implicitly setting it to the location of the instruction before it, is not the right thing to do. It may have the desired effect in your example, but it't not generally correct.

Aug 10 2020, 11:37 AM · Restricted Project, debug-info

# Jul 31 2020

Hi @riccibruno,
Name :- Jaydeep Chauhan
Mail id:- jaydeepchauhan1494@gmail.com
Thanks

Jul 31 2020, 10:52 AM · Restricted Project

# Jul 30 2020

Hi @aaron.ballman ,
Thank you for accepting this. I don't have commit access please commit this.
Thanks.

Jul 30 2020, 6:38 AM · Restricted Project

# Jul 29 2020

This patch covers this scenario also.
Thanks @aaron.ballman

Jul 29 2020, 9:55 AM · Restricted Project

# Jul 28 2020

Jul 28 2020, 10:35 PM · Restricted Project

Jul 28 2020, 5:48 AM · Restricted Project
Jac1494 added inline comments to D84678: [clang] False line number in a function definition with "void" parameter.
Jul 28 2020, 12:32 AM · Restricted Project

# Jul 27 2020

Jul 27 2020, 10:16 AM · Restricted Project

# Jul 24 2020

Jac1494 added a comment to D83468: [Debuginfo] Fix for PR46653.

Hi @aprantl , Thank you for reply.

Jul 24 2020, 3:40 AM · Restricted Project, debug-info

# Jul 23 2020

Jac1494 added a comment to D83468: [Debuginfo] Fix for PR46653.

ping?

Jul 23 2020, 9:30 AM · Restricted Project, debug-info

# Jul 21 2020

Jul 21 2020, 12:27 PM · Restricted Project, debug-info

# Jul 16 2020

Jac1494 added reviewers for D83468: [Debuginfo] Fix for PR46653: .
Jul 16 2020, 3:30 AM · Restricted Project, debug-info

# Jul 15 2020

Jac1494 updated the diff for D83468: [Debuginfo] Fix for PR46653.

Issue is with Global-isel and specific to AAarch64.

Jul 15 2020, 12:33 PM · Restricted Project, debug-info

# Jul 14 2020

Jul 14 2020, 9:59 AM · Restricted Project, debug-info
Jac1494 updated the summary of D83468: [Debuginfo] Fix for PR46653.
Jul 14 2020, 9:56 AM · Restricted Project, debug-info

# Jul 9 2020

Herald added a project to D83468: [Debuginfo] Fix for PR46653: Restricted Project.
Jul 9 2020, 2:44 AM · Restricted Project, debug-info

# Apr 16 2020

Sure. Would you like me to use the email address you use on bugzilla as the commit author? If not, could you supply an address to use?

Apr 16 2020, 11:41 AM · Restricted Project

Thank you for accepting this. I don't have commit access please commit this.

Apr 16 2020, 11:08 AM · Restricted Project
Apr 16 2020, 10:35 AM · Restricted Project

# Feb 5 2020

ping

Feb 5 2020, 4:46 AM · Restricted Project, Restricted Project, debug-info

# Jan 27 2020

ping

I'd be curious to the answer to David's questions. If the size increase is because of unused extern variables coming in from libc or something then it doesn't seem worth the cost.

Jan 27 2020, 5:29 AM · Restricted Project, Restricted Project, debug-info
Jac1494 added inline comments to D71451: Support to emit extern variables debuginfo with "-fstandalone-debug".
Jan 27 2020, 5:17 AM · Restricted Project, Restricted Project, debug-info

# Jan 26 2020

Jac1494 added inline comments to D71599: [LLVM] Support to emit extern variables debuginfo with "-fstandalone-debug".
Jan 26 2020, 10:43 PM · Restricted Project

# Jan 22 2020

Jac1494 updated the summary of D71599: [LLVM] Support to emit extern variables debuginfo with "-fstandalone-debug".
Jan 22 2020, 12:05 PM · Restricted Project

clang format source file added ,and removed unnecessary attribute from test case.

Jan 22 2020, 11:47 AM · Restricted Project

# Jan 20 2020

Jan 20 2020, 7:03 AM · Restricted Project
Jac1494 updated the diff for D71451: Support to emit extern variables debuginfo with "-fstandalone-debug".
Jan 20 2020, 6:54 AM · Restricted Project, Restricted Project, debug-info
Jac1494 updated the diff for D71451: Support to emit extern variables debuginfo with "-fstandalone-debug".
Jan 20 2020, 5:40 AM · Restricted Project, Restricted Project, debug-info

# Jan 19 2020

that still seems very surprising - this change is specifically intended to add more debug info, is it not? So do you have any ideas/theories as to why that's not showing up in the data?

Jan 19 2020, 9:46 PM · Restricted Project, Restricted Project, debug-info

Do you have any reason to believe these patches would reduce the size of the debug info? It seems like they only add more debug info, and don't remove anything - so we'd expect an increase in the size, certainly not a decrease. That means, to me, there's probably a mistake in the measurement somehow?

Jan 19 2020, 11:30 AM · Restricted Project, Restricted Project, debug-info
Jan 19 2020, 11:11 AM · Restricted Project

Jan 19 2020, 11:07 AM · Restricted Project

# Jan 15 2020

I'd be curious to the answer to David's questions. If the size increase is because of unused extern variables coming in from libc or something then it doesn't seem worth the cost.

Jan 15 2020, 11:37 AM · Restricted Project, Restricted Project, debug-info

# Jan 12 2020

ping

Jan 12 2020, 11:11 PM · Restricted Project, Restricted Project, debug-info

# Dec 21 2019

Am I reading this right that the data would suggest that enabling this feature /reduces/ the size of debug info sections? That doesn't sound right - can you explain why that would be the case? (perhaps the data is incorrect/it wasn't built with -fstandalone-debug?)

Dec 21 2019, 1:37 AM · Restricted Project, Restricted Project, debug-info

# Dec 19 2019

Dec 19 2019, 10:42 AM · Restricted Project

@Jac1494 - have you made any measurements of the size increase of this change? Perhaps a self-host build of clang?

Dec 19 2019, 8:41 AM · Restricted Project, Restricted Project, debug-info

# Dec 17 2019

Do you have any particular users/use case for this?

Dec 17 2019, 5:13 AM · Restricted Project, Restricted Project, debug-info
Dec 17 2019, 4:35 AM · Restricted Project
Jac1494 updated the diff for D71451: Support to emit extern variables debuginfo with "-fstandalone-debug".

Separate clang patch with test cases .

Dec 17 2019, 4:35 AM · Restricted Project, Restricted Project, debug-info

# Dec 15 2019

Dec 15 2019, 10:25 PM · Restricted Project, Restricted Project, debug-info
Jac1494 updated the summary of D71451: Support to emit extern variables debuginfo with "-fstandalone-debug".
Dec 15 2019, 10:18 PM · Restricted Project, Restricted Project, debug-info

# Dec 12 2019

Dec 12 2019, 11:37 PM · Restricted Project, Restricted Project, debug-info

# Dec 9 2019

Jac1494 added a comment to D70696: [DebugInfo] Support to emit debugInfo for extern variables.

For some real life case like below we need debuginfo for declaration of global extern variable .

Dec 9 2019, 4:13 AM · Restricted Project, Restricted Project, debug-info