Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 25 Jul 2023 13:51:06 +0200
From:   Peter Zijlstra <peterz@infradead.org>
To:     Tiezhu Yang <yangtiezhu@loongson.cn>
Cc:     Josh Poimboeuf <jpoimboe@kernel.org>,
        Huacai Chen <chenhuacai@kernel.org>, loongarch@lists.linux.dev,
        linux-kernel@vger.kernel.org, loongson-kernel@lists.loongnix.cn
Subject: Re: [PATCH v1 4/6] objtool/LoongArch: Enable orc to be built
Message-ID: <20230725115106.GC3765278@hirez.programming.kicks-ass.net>
References: <1690272910-11869-1-git-send-email-yangtiezhu@loongson.cn>
 <1690272910-11869-5-git-send-email-yangtiezhu@loongson.cn>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1690272910-11869-5-git-send-email-yangtiezhu@loongson.cn>
Precedence: bulk

On Tue, Jul 25, 2023 at 04:15:08PM +0800, Tiezhu Yang wrote:

Previous Changelog had:

> Define update_cfi_state() as a weak function which may be overwritten
> by the arch-specific implementation.

And then this patch does:

> +int update_cfi_state(struct instruction *insn,
> +		     struct instruction *next_insn,
> +		     struct cfi_state *cfi, struct stack_op *op)
> +{
> +	struct cfi_reg *cfa = &cfi->cfa;
> +	struct cfi_reg *regs = cfi->regs;
> +
> +	/* ignore UNWIND_HINT_UNDEFINED regions */
> +	if (cfi->force_undefined)
> +		return 0;
> +
> +	/* stack operations don't make sense with an undefined CFA */
> +	if (cfa->base == CFI_UNDEFINED) {
> +		if (insn_func(insn)) {
> +			WARN_INSN(insn, "undefined stack state");
> +			return -1;
> +		}
> +		return 0;
> +	}
> +
> +	if (cfi->type == UNWIND_HINT_TYPE_REGS ||
> +	    cfi->type == UNWIND_HINT_TYPE_REGS_PARTIAL)
> +		return update_cfi_state_regs(insn, cfi, op);
> +
> +	switch (op->dest.type) {
> +	case OP_DEST_REG:
> +		switch (op->src.type) {
> +		case OP_SRC_ADD:
> +			if (op->dest.reg == CFI_SP && op->src.reg == CFI_SP) {
> +				/* addi.d sp,sp,si12 */
> +				cfi->stack_size -= op->src.offset;
> +				if (cfa->base == CFI_SP)
> +					cfa->offset -= op->src.offset;
> +			} else if (op->dest.reg == CFI_FP && op->src.reg == CFI_SP) {
> +				/* addi.d fp,sp,si12 */
> +				if (cfa->base == CFI_SP && cfa->offset == op->src.offset) {
> +					cfa->base = CFI_FP;
> +					cfa->offset = 0;
> +				}
> +			} else if (op->dest.reg == CFI_SP && op->src.reg == CFI_FP) {
> +				/* addi.d sp,fp,si12 */
> +				if (cfa->base == CFI_FP && cfa->offset == 0) {
> +					cfa->base = CFI_SP;
> +					cfa->offset = -op->src.offset;
> +				}
> +			}
> +			break;
> +		case OP_SRC_REG_INDIRECT:
> +			/* ld.d rd,sp,si12 */
> +			if (op->src.reg == CFI_SP &&
> +			    op->src.offset == (regs[op->dest.reg].offset + cfi->stack_size)) {
> +				restore_reg(cfi, op->dest.reg);
> +				/* GCC may not restore sp, we adjust it directly. */
> +				if (cfa->base == CFI_FP && cfa->offset == 0) {
> +					cfa->base = CFI_SP;
> +					cfa->offset = cfi->stack_size;
> +				}
> +			}
> +			break;
> +		default:
> +			break;
> +		}
> +		break;
> +	case OP_DEST_REG_INDIRECT:
> +		if (op->src.type == OP_SRC_REG)
> +			/* st.d rd,sp,si12 */
> +			if (op->dest.offset)
> +				save_reg(cfi, op->src.reg, CFI_CFA,
> +					 op->dest.offset - cfi->stack_size);
> +		break;
> +	default:
> +		WARN_FUNC("unknown stack-related instruction", insn->sec, insn->offset);
> +		return -1;
> +	}
> +
> +	return 0;
> +}

Why ?!? what is the actual irreconcilable difference?

If you want us to review this, you'll have to explain things.