Page MenuHomePhabricator

gberry (Geoff Berry)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 29 2015, 2:08 PM (233 w, 3 d)

Recent Activity

Jun 15 2018

gberry committed rL334857: Update my information in the CREDITS file..
Update my information in the CREDITS file.
Jun 15 2018, 1:08 PM

May 24 2018

gberry committed rL333223: Merging r330976:.
Merging r330976:
May 24 2018, 1:53 PM
gberry committed rL333214: [AArch64] Take advantage of variable shift/rotate amount implicit mod operation..
[AArch64] Take advantage of variable shift/rotate amount implicit mod operation.
May 24 2018, 11:33 AM
gberry closed D46844: [AArch64] Take advantage of variable shift/rotate amount implicit mod operation..
May 24 2018, 11:33 AM

May 21 2018

gberry added a comment to D46844: [AArch64] Take advantage of variable shift/rotate amount implicit mod operation..

Ping?

May 21 2018, 12:35 PM

May 15 2018

gberry committed rL332367: [AArch64] Fix mir test case liveins info..
[AArch64] Fix mir test case liveins info.
May 15 2018, 9:31 AM

May 14 2018

gberry created D46844: [AArch64] Take advantage of variable shift/rotate amount implicit mod operation..
May 14 2018, 1:11 PM
gberry committed rL332265: [BranchFolding] Allow hoisting to block with a single conditional branch..
[BranchFolding] Allow hoisting to block with a single conditional branch.
May 14 2018, 10:37 AM
gberry closed D46324: [BranchFolding] Allow hoisting to block with a single conditional branch..
May 14 2018, 10:37 AM
gberry added a comment to D46324: [BranchFolding] Allow hoisting to block with a single conditional branch..

One thought I had was: if there is a block with only an unconditional branch, should we fold it away first? That is, redirect any branches to that block to the target of the branch?

May 14 2018, 8:42 AM

May 11 2018

gberry updated the diff for D46324: [BranchFolding] Allow hoisting to block with a single conditional branch..

Add some comments

May 11 2018, 11:47 AM
gberry committed rL332103: [AArch64] Fix performPostLD1Combine to check for constant lane index..
[AArch64] Fix performPostLD1Combine to check for constant lane index.
May 11 2018, 9:29 AM
gberry closed D46591: [AArch64] Fix performPostLD1Combine to check for constant lane index..
May 11 2018, 9:28 AM
gberry added a comment to D46324: [BranchFolding] Allow hoisting to block with a single conditional branch..

Ping?

May 11 2018, 7:42 AM

May 9 2018

gberry added inline comments to D46591: [AArch64] Fix performPostLD1Combine to check for constant lane index..
May 9 2018, 1:49 PM
gberry updated the diff for D46591: [AArch64] Fix performPostLD1Combine to check for constant lane index..

Add lane index range check

May 9 2018, 1:46 PM

May 8 2018

gberry created D46591: [AArch64] Fix performPostLD1Combine to check for constant lane index..
May 8 2018, 10:59 AM

May 4 2018

gberry committed rL331549: [MachineLICM] Debug intrinsics shouldn't affect hoist decisions.
[MachineLICM] Debug intrinsics shouldn't affect hoist decisions
May 4 2018, 12:31 PM
gberry closed D46284: [MachineLICM] Debug intrinsics shouldn't affect hoist decisions.
May 4 2018, 12:31 PM

May 1 2018

gberry created D46324: [BranchFolding] Allow hoisting to block with a single conditional branch..
May 1 2018, 11:07 AM

Apr 30 2018

gberry added a reviewer for D46284: [MachineLICM] Debug intrinsics shouldn't affect hoist decisions: davide.
Apr 30 2018, 3:16 PM
gberry created D46284: [MachineLICM] Debug intrinsics shouldn't affect hoist decisions.
Apr 30 2018, 1:26 PM
gberry added a comment to D45878: [DEBUG INFO] Fixing cases where debug info (-g) causes changes in the program..

I started to write the utility functions in MachineInstr/MachineBasicBlock, but thinking about it, I wonder if it wouldn't be even better to integrate fully the handling of DBG_VALUEs and DebugLocs into splice()? After all, the DBG_VALUEs should *always* be (re)moved together with the MachineInstr, so why expose this to a particular optimizer pass? If splice() would do this per default, the optimizer would not have to worry about it, and could not even forget about it. Is there a reason to always do this on the side?

If this is guarded by a check to see if the following instruction is a DBG_VALUE, I think builds without debug info would not suffer from increased compile-time. BTW, is there a better way to check if it is a debug build or not, like maybe hasDebugInfo()?

One issue with this is that the caller usually has an iterator, that now gets invalidated if a DBG_VALUE is moved automatically. I think this means splice() has to return an iterator to the next following instruction after the moved one(s). There would also have to be one method that handled reverse iterators. My idea is that splice would only do this if an iterator is passed as an optional argument, so that optimizers could gradually get used to this.

Again, I am not an expert here, just assuming from quick observation that when moving an instruction it is good to

  1. move any connected DBG_VALUEs with the moved MI
  2. update the DebugLoc of MI for its new position.

    I have so far just reused the code in MachineSink, but I don't know how good this is or if it could be improved. MachineSink is currently checking through all DBG_VALUEs after MI and picking only the right ones, and stopping at first non-debug MI. This makes sense to me, even though I'm not really clear on why the DBG_VALUEs could end up mixed like that. Could it perhaps be that if we started handling this properly everywhere, we could assume that they are never mixed (that DBG_VALUE that express a use of a register is always to be found directly after the defining MI)? Does this relate perhaps to instructions with multiple defs (like an address post-increment), and if so, should we perhaps scan MI for all explicit defs instead of just the one at index 0?

    Of course, Pre-RA this could be done with MRI use-def information instead of the search.

    Then there is the question of what to do when moving a range of instructions. Depending on how mixed up the DBG_VALUEs might get (when all optimizers are doing the right thing), one might want to either handle the whole range of instructions to find out what following DBG_VALUEs to also move, or perhaps just look at the last instruction of the range.

    Similarly, eraseFromParent() could always also erase any connected DBG_VALUes. There is actually eraseFromParentAndMarkDBGValuesForRemoval, but only with virtual registers, sofar. Should the search for DBG_VALUES after MI be added here to be done post-RA?

    I would like to see some clear comment on exactly what is expected from an optimization pass that moves / erases instructions in regards to DBG_VALUE instructions, but can't find one...?

    Do you think this makes sense, or should we stick with separate utility functions?
Apr 30 2018, 7:23 AM

Apr 26 2018

gberry committed rL330976: [AArch64] Fix scavenged spill slot base when stack realignment required..
[AArch64] Fix scavenged spill slot base when stack realignment required.
Apr 26 2018, 11:54 AM
gberry closed D46063: [AArch64] Fix scavenged spill slot base when stack realignment required..
Apr 26 2018, 11:54 AM
gberry added a comment to D45858: [DivRemPairs] Fix non-determinism in use list order..

A quick background on the reverse-iteration bot: https://llvm.org/docs/CodingStandards.html#beware-of-non-determinism-due-to-ordering-of-pointers
Unordered containers like SmallPtrSet and DenseMap do not guarantee the order of iteration of elements. So iterating them can result in non-deterministic codegen. However, not all instances of iterating an unordered container result in non-determinism. Given this, it becomes difficult to identify which iteration instances can cause this behavior. The reverse iteration bot iterates such containers in reverse, by default, to weed out such cases.

However, the bot is only useful only if there are tests which stress/depend on iteration order. And while it may be possible to write such tests I am not sure if it's worth the effort. My experience also has been that I have randomly run into non-deterministic behavior and debugging led me to problems in iteration order.

Apr 26 2018, 8:58 AM
gberry added a comment to D45878: [DEBUG INFO] Fixing cases where debug info (-g) causes changes in the program..

I've been looking into this issue for AArch64 recently as well (see bug 37240 for a PostRA scheduler issue I found). I'll be filing more bugs and/or posting fixes for other cases that we hit on our target with our different compiler flags in the next few days.

Apr 26 2018, 7:57 AM

Apr 25 2018

gberry added a comment to D45770: [AArch64] Disable spill slot scavenging when stack realignment required..

I've put the full diff for my alternative fix here: D46063

Apr 25 2018, 8:14 AM
gberry created D46063: [AArch64] Fix scavenged spill slot base when stack realignment required..
Apr 25 2018, 8:13 AM

Apr 24 2018

gberry committed rL330792: [DivRemPairs] Fix non-determinism in use list order..
[DivRemPairs] Fix non-determinism in use list order.
Apr 24 2018, 7:21 PM
gberry closed D45858: [DivRemPairs] Fix non-determinism in use list order..
Apr 24 2018, 7:21 PM

Apr 23 2018

gberry added a comment to D45858: [DivRemPairs] Fix non-determinism in use list order..

If there is a unit test for this then it should have been uncovered in the reverse iteration bot: http://lab.llvm.org:8011/builders/reverse-iteration

Apr 23 2018, 12:29 PM
gberry added a comment to D45858: [DivRemPairs] Fix non-determinism in use list order..

That looks ok, but I'm curious;

  1. How do you discover this problem?
Apr 23 2018, 10:46 AM

Apr 19 2018

gberry created D45858: [DivRemPairs] Fix non-determinism in use list order..
Apr 19 2018, 7:35 PM
gberry added a comment to D45770: [AArch64] Disable spill slot scavenging when stack realignment required..

Ugh, good find. Isn't this an issue if variable sized stack objects are present as well?

The graphic (within the same file) showing the stack layout suggests a base pointer is used in such circumstances, so my guess is no.

Apr 19 2018, 3:17 PM

Apr 18 2018

gberry added a comment to D45770: [AArch64] Disable spill slot scavenging when stack realignment required..

@MatzeB This isn't about the scavenging of a register to use for large offsets, etc., but rather the scavenging of stack slots in the CSR save area to use for regular stack objects.

Apr 18 2018, 10:14 AM
gberry added reviewers for D45770: [AArch64] Disable spill slot scavenging when stack realignment required.: t.p.northover, MatzeB.

Ugh, good find. Isn't this an issue if variable sized stack objects are present as well?

Apr 18 2018, 9:03 AM

Apr 10 2018

gberry committed rL329761: [AArch64][Falkor] Fix bug in Falkor HWPF collision avoidance pass..
[AArch64][Falkor] Fix bug in Falkor HWPF collision avoidance pass.
Apr 10 2018, 2:46 PM
gberry closed D45502: [AArch64][Falkor] Fix bug in Falkor HWPF collision avoidance pass..
Apr 10 2018, 2:46 PM
gberry created D45502: [AArch64][Falkor] Fix bug in Falkor HWPF collision avoidance pass..
Apr 10 2018, 1:26 PM

Apr 6 2018

gberry committed rL329443: [EarlyCSE] Add debug counter for debugging mis-optimizations. NFC..
[EarlyCSE] Add debug counter for debugging mis-optimizations. NFC.
Apr 6 2018, 11:50 AM
gberry closed D45162: [EarlyCSE] Add debug counter for debugging mis-optimizations. NFC..
Apr 6 2018, 11:50 AM

Apr 3 2018

gberry added a reviewer for D45189: [MachineOutliner][AArch64] Keep track of functions that use a red zone in AArch64MachineFunctionInfo and use that instead of checking for noredzone in the MachineOutliner: MatzeB.
Apr 3 2018, 7:34 AM
gberry added a comment to D45189: [MachineOutliner][AArch64] Keep track of functions that use a red zone in AArch64MachineFunctionInfo and use that instead of checking for noredzone in the MachineOutliner.

Another thought, would it be worthwhile to add a serialized MIR function property for this usesRedZone flag so you could write MIR tests for this instead?

Apr 3 2018, 7:34 AM
gberry added inline comments to D45189: [MachineOutliner][AArch64] Keep track of functions that use a red zone in AArch64MachineFunctionInfo and use that instead of checking for noredzone in the MachineOutliner.
Apr 3 2018, 7:27 AM

Apr 2 2018

gberry added a comment to D39976: [AArch64] Query the target when folding loads and stores.

FeatureSlowPaired128 was just too coarse. The alternative would be to change it to something more specific, like FeatureSlowSomePaired128Sometimes, and then create yet another when for the next generation to specialize it further. Instead, querying the scheduling model seems to be a much more reasonable approach.

I'm more confused now. 'FeatureSlowPaired128' controls whether certain load/store opcodes are combined to form paired load/stores. But this change prevents some load/store opcodes from having their base register increment folded in. The two seem unrelated.

This change is more generic and flexible than FeatureSlowPaired128. This change controls not only when loads and stores are paired, but also other foldings that this pass performs, including the pre or post indexing of the offset register.

Apr 2 2018, 10:01 AM
gberry updated the diff for D45162: [EarlyCSE] Add debug counter for debugging mis-optimizations. NFC..

Add test

Apr 2 2018, 9:31 AM
gberry created D45162: [EarlyCSE] Add debug counter for debugging mis-optimizations. NFC..
Apr 2 2018, 7:09 AM

Mar 28 2018

gberry accepted D44911: [MachineCopyPropagation] Handle COPY with overlapping source/dest..

I get it now. LGTM

Mar 28 2018, 3:12 PM
gberry added a comment to D44911: [MachineCopyPropagation] Handle COPY with overlapping source/dest..

I'm not sure I see what's wrong with removing the second COPY in either of these cases, unless the semantics of the multi-register COPY are that each component register is copied in order?

Mar 28 2018, 1:36 PM

Mar 20 2018

gberry committed rL327982: [AArch64][Falkor] Correct load/store increment scheduling details.
[AArch64][Falkor] Correct load/store increment scheduling details
Mar 20 2018, 6:49 AM

Mar 16 2018

gberry added a comment to D39976: [AArch64] Query the target when folding loads and stores.

Methinks that the gist is to move away from features and to rely more on the cost model. In the case of this patch, it also removes the feature FeatureSlowPaired128 in D40107.

That seems like a worthwhile goal, but this change doesn't really seem to be accomplishing that. If the sched model is being used by a subtarget-specific heuristic, that seems like just a more roundabout way of achieving the same result for your subtarget. Is there any net effect of this change combined with D40107?

FeatureSlowPaired128 was just too coarse. The alternative would be to change it to something more specific, like FeatureSlowSomePaired128Sometimes, and then create yet another when for the next generation to specialize it further. Instead, querying the scheduling model seems to be a much more reasonable approach.

Mar 16 2018, 11:00 AM

Mar 5 2018

gberry added a comment to D39976: [AArch64] Query the target when folding loads and stores.

Would it not be simpler to just add a subtarget bool that controls whether the problematic opcodes are emitted and set it for your subtargets (similar to the way STRQroIsSlow is handled)? That way you could avoid generating them not just in this pass, but in ISel and frame lowering?

Methinks that the gist is to move away from features and to rely more on the cost model. In the case of this patch, it also removes the feature FeatureSlowPaired128 in D40107.

Mar 5 2018, 11:06 AM

Feb 27 2018

gberry committed rL326208: Re-enable "[MachineCopyPropagation] Extend pass to do COPY source forwarding".
Re-enable "[MachineCopyPropagation] Extend pass to do COPY source forwarding"
Feb 27 2018, 9:02 AM

Feb 23 2018

gberry committed rL325938: Fix compiler warning introduced in r325931. NFC..
Fix compiler warning introduced in r325931. NFC.
Feb 23 2018, 11:13 AM
gberry committed rL325931: [MachineOperand][Target] MachineOperand::isRenamable semantics changes.
[MachineOperand][Target] MachineOperand::isRenamable semantics changes
Feb 23 2018, 10:29 AM
gberry closed D43042: [MachineOperand][Target] MachineOperand::isRenamable semantics changes.
Feb 23 2018, 10:29 AM
gberry added a comment to D39976: [AArch64] Query the target when folding loads and stores.

Would it not be simpler to just add a subtarget bool that controls whether the problematic opcodes are emitted and set it for your subtargets (similar to the way STRQroIsSlow is handled)? That way you could avoid generating them not just in this pass, but in ISel and frame lowering?

Feb 23 2018, 7:29 AM

Feb 22 2018

gberry updated the diff for D43042: [MachineOperand][Target] MachineOperand::isRenamable semantics changes.

Add test/CodeGen/AMDGPU/postra-norename.mir as requested by Quentin

Feb 22 2018, 10:47 AM

Feb 21 2018

gberry retitled D43042: [MachineOperand][Target] MachineOperand::isRenamable semantics changes from [MachineOperand][Target] Add target option to disable setting MachineOperand::isRenamable to [MachineOperand][Target] MachineOperand::isRenamable semantics changes.
Feb 21 2018, 12:35 PM
gberry added a reviewer for D43042: [MachineOperand][Target] MachineOperand::isRenamable semantics changes: arsenm.

@arsenm Could you take a look at the AMDGPU change?

Feb 21 2018, 12:35 PM
gberry updated the diff for D43042: [MachineOperand][Target] MachineOperand::isRenamable semantics changes.

New version based on review feedback.

Feb 21 2018, 12:31 PM

Feb 12 2018

gberry added a comment to D43042: [MachineOperand][Target] MachineOperand::isRenamable semantics changes.

Checking hasExtraRegAllocReg in isRenamable sounds reasonable to me (though it may require the interface to be changed slightly since we'll need the opcode). An alternative that is closer to what I initially proposed would be to have setReg and addOperand both clear the isRenamable flag. This way the only operands that would be renamable are those that have been untouched since they were marked renamable by the register allocator. This would also allow us to get rid of the extra code that I added to clear the renamable flag in some post-RA target code. Targets would be able to opt in to preserving the renamable flag by correctly preserving the renamable flag in post-RA transformed code, but by default the flag would be set conservatively. I'm going to try this approach out to see what it looks like and what kind of impact it has unless someone has a reason they think it won't work.

Feb 12 2018, 10:44 AM

Feb 7 2018

gberry created D43042: [MachineOperand][Target] MachineOperand::isRenamable semantics changes.
Feb 7 2018, 1:38 PM

Feb 5 2018

gberry added a comment to D39976: [AArch64] Query the target when folding loads and stores.

I've thought about this some more and tested it out on Falkor. As currently written this change causes SIMD store instructions to not have pre/post increments folded into them, causing minor performance regressions. I have the following general reservations as well:

  • does using the max latency of the load/store and add make sense given that the operations are dependent?
  • does always favoring latency over number of uops (an approximation of throughput) make sense? unless the operation is on the critical path I would think not.
Feb 5 2018, 11:43 AM

Feb 1 2018

gberry committed rL323991: [MachineCopyPropagation] Extend pass to do COPY source forwarding.
[MachineCopyPropagation] Extend pass to do COPY source forwarding
Feb 1 2018, 10:56 AM
gberry closed D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding.
Feb 1 2018, 10:56 AM

Jan 31 2018

gberry committed rL323905: [MachineOutliner] Freeze registers in new functions.
[MachineOutliner] Freeze registers in new functions
Jan 31 2018, 12:17 PM
gberry closed D42749: [MachineOutliner] Freeze registers in new functions.
Jan 31 2018, 12:17 PM
gberry created D42749: [MachineOutliner] Freeze registers in new functions.
Jan 31 2018, 10:46 AM

Jan 30 2018

gberry committed rL323794: [AMDGPU] isRenamable fixes to support copy forwarding.
[AMDGPU] isRenamable fixes to support copy forwarding
Jan 30 2018, 9:39 AM

Jan 29 2018

gberry committed rL323676: [MachineVerifier] Add check that renamable operands aren't reserved registers..
[MachineVerifier] Add check that renamable operands aren't reserved registers.
Jan 29 2018, 10:58 AM
gberry closed D42449: [MachineVerifier] Add check that renamable operands aren't reserved registers..
Jan 29 2018, 10:58 AM
gberry committed rL323675: [AMDGPU][X86][Mips] Make sure renamable bit not set for reserved regs.
[AMDGPU][X86][Mips] Make sure renamable bit not set for reserved regs
Jan 29 2018, 10:51 AM
gberry added inline comments to D42449: [MachineVerifier] Add check that renamable operands aren't reserved registers..
Jan 29 2018, 8:49 AM
gberry updated the diff for D42449: [MachineVerifier] Add check that renamable operands aren't reserved registers..

Add comments to address feedback from Quentin

Jan 29 2018, 8:49 AM

Jan 24 2018

gberry added a comment to D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding.

D42449 adds the machine verifier check that no renamable operands are assigned to reserved registers

Jan 24 2018, 12:00 PM
gberry added a parent revision for D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding: D42449: [MachineVerifier] Add check that renamable operands aren't reserved registers..
Jan 24 2018, 11:57 AM
gberry added a child revision for D42449: [MachineVerifier] Add check that renamable operands aren't reserved registers.: D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding.
Jan 24 2018, 11:57 AM
gberry added a reviewer for D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding: arsenm.
Jan 24 2018, 11:57 AM
gberry added a comment to D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding.

I've uploaded a new version that addresses most of the issues brought up by @qcolombet.

Jan 24 2018, 11:57 AM
gberry updated the diff for D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding.

Updates based on Quentin's comments

Jan 24 2018, 11:53 AM
gberry committed rL323356: [AMDGPU] Make sure all super regs of reserved regs are marked reserved..
[AMDGPU] Make sure all super regs of reserved regs are marked reserved.
Jan 24 2018, 10:12 AM
gberry closed D42448: [AMDGPU] Make sure all super regs of reserved regs are marked reserved..
Jan 24 2018, 10:12 AM

Jan 23 2018

gberry added a comment to D42448: [AMDGPU] Make sure all super regs of reserved regs are marked reserved..

Thanks!

Jan 23 2018, 3:29 PM
gberry created D42449: [MachineVerifier] Add check that renamable operands aren't reserved registers..
Jan 23 2018, 3:16 PM
gberry created D42448: [AMDGPU] Make sure all super regs of reserved regs are marked reserved..
Jan 23 2018, 3:12 PM

Jan 19 2018

gberry added inline comments to D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding.
Jan 19 2018, 2:33 PM

Jan 16 2018

gberry added a comment to D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding.

Ping?

Jan 16 2018, 11:51 AM

Jan 11 2018

gberry added reviewers for D39976: [AArch64] Query the target when folding loads and stores: MatzeB, t.p.northover.
Jan 11 2018, 12:04 PM

Jan 8 2018

gberry added a comment to D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding.

@tstellar could you specifically take a look at the AMDGPU backend changes?

Jan 8 2018, 1:17 PM
gberry created D41835: [MachineCopyPropagation] Extend pass to do COPY source forwarding.
Jan 8 2018, 1:15 PM

Jan 3 2018

gberry added a comment to D40876: AArch64: Fix emergency spillslot being out of reach for large callframes.

I'm not sure using the redzone is safe on all targets.

Jan 3 2018, 1:42 PM
gberry added a comment to D40876: AArch64: Fix emergency spillslot being out of reach for large callframes.

Okay, I get it now, I had the wrong case in mind. I was thinking that you could put the scavenge slot close to [sp], but that doesn't work since you have a large outgoing stack parameter area.
This change looks good to me, but you might want to get a second opinion since my thinking on this hasn't been that clear.

Jan 3 2018, 11:31 AM
gberry added a comment to D40876: AArch64: Fix emergency spillslot being out of reach for large callframes.

I think what is bothering me about this change is that the return value of hasFP() now seems more dynamic. Did you consider a potentially simpler fix of just creating the spill slot as a fixed stack object with a hard-coded offset that would guarantee it is directly addressable from the SP/FP?

Jan 3 2018, 8:53 AM

Dec 29 2017

gberry committed rL321571: [MachineOperand] Fix LiveDebugVariables code after isRenamable change..
[MachineOperand] Fix LiveDebugVariables code after isRenamable change.
Dec 29 2017, 1:02 PM

Dec 22 2017

gberry abandoned D23366: [AArch64] Assign stack locations to increase load/store pairing..

I'll resubmit this for review if I ever get back around to it

Dec 22 2017, 12:41 PM

Dec 18 2017

gberry added a comment to D40876: AArch64: Fix emergency spillslot being out of reach for large callframes.

One thing I noticed when looking at this: it seems fragile to me to have MachineFrameInfo::computeMaxCallFrameSize() and PEI::calculateFrameInfo() doing essentially the same calculation with duplicated code.
Would it make sense to add an assert to PEI::calculateFrameInfo that checks that the previously calculated MaxCallFrameSize isn't smaller than the one calculated later?

Dec 18 2017, 1:04 PM
gberry added a comment to D40876: AArch64: Fix emergency spillslot being out of reach for large callframes.

One thing I noticed when looking at this: it seems fragile to me to have MachineFrameInfo::computeMaxCallFrameSize() and PEI::calculateFrameInfo() doing essentially the same calculation with duplicated code.
Would it make sense to add an assert to PEI::calculateFrameInfo that checks that the previously calculated MaxCallFrameSize isn't smaller than the one calculated later?

Dec 18 2017, 11:59 AM
gberry added inline comments to D39976: [AArch64] Query the target when folding loads and stores.
Dec 18 2017, 11:21 AM